Merge pull request #998 from integer32llc/links-improvements

Refactoring of some functionality in links.rs

src/preprocess/links.rs

@@ -2,7 +2,7 @@ use crate::errors::*;
 use crate::utils::{take_anchored_lines, take_lines};
 use regex::{CaptureMatches, Captures, Regex};
 use std::fs;
-use std::ops::{Range, RangeFrom, RangeFull, RangeTo};
+use std::ops::{Bound, Range, RangeBounds, RangeFrom, RangeFull, RangeTo};
 use std::path::{Path, PathBuf};
 
 use super::{Preprocessor, PreprocessorContext};
@@ -102,23 +102,70 @@ where
 #[derive(PartialEq, Debug, Clone)]
 enum LinkType<'a> {
     Escaped,
-    IncludeRange(PathBuf, Range<usize>),
+    IncludeRange(PathBuf, LineRange),
-    IncludeRangeFrom(PathBuf, RangeFrom<usize>),
-    IncludeRangeTo(PathBuf, RangeTo<usize>),
-    IncludeRangeFull(PathBuf, RangeFull),
     IncludeAnchor(PathBuf, String),
     Playpen(PathBuf, Vec<&'a str>),
 }
 
+// A range of lines specified with some include directive.
+#[derive(PartialEq, Debug, Clone)]
+enum LineRange {
+    Range(Range<usize>),
+    RangeFrom(RangeFrom<usize>),
+    RangeTo(RangeTo<usize>),
+    RangeFull(RangeFull),
+}
+
+impl RangeBounds<usize> for LineRange {
+    fn start_bound(&self) -> Bound<&usize> {
+        match self {
+            LineRange::Range(r) => r.start_bound(),
+            LineRange::RangeFrom(r) => r.start_bound(),
+            LineRange::RangeTo(r) => r.start_bound(),
+            LineRange::RangeFull(r) => r.start_bound(),
+        }
+    }
+
+    fn end_bound(&self) -> Bound<&usize> {
+        match self {
+            LineRange::Range(r) => r.end_bound(),
+            LineRange::RangeFrom(r) => r.end_bound(),
+            LineRange::RangeTo(r) => r.end_bound(),
+            LineRange::RangeFull(r) => r.end_bound(),
+        }
+    }
+}
+
+impl From<Range<usize>> for LineRange {
+    fn from(r: Range<usize>) -> LineRange {
+        LineRange::Range(r)
+    }
+}
+
+impl From<RangeFrom<usize>> for LineRange {
+    fn from(r: RangeFrom<usize>) -> LineRange {
+        LineRange::RangeFrom(r)
+    }
+}
+
+impl From<RangeTo<usize>> for LineRange {
+    fn from(r: RangeTo<usize>) -> LineRange {
+        LineRange::RangeTo(r)
+    }
+}
+
+impl From<RangeFull> for LineRange {
+    fn from(r: RangeFull) -> LineRange {
+        LineRange::RangeFull(r)
+    }
+}
+
 impl<'a> LinkType<'a> {
     fn relative_path<P: AsRef<Path>>(self, base: P) -> Option<PathBuf> {
         let base = base.as_ref();
         match self {
             LinkType::Escaped => None,
             LinkType::IncludeRange(p, _) => Some(return_relative_path(base, &p)),
-            LinkType::IncludeRangeFrom(p, _) => Some(return_relative_path(base, &p)),
-            LinkType::IncludeRangeTo(p, _) => Some(return_relative_path(base, &p)),
-            LinkType::IncludeRangeFull(p, _) => Some(return_relative_path(base, &p)),
             LinkType::IncludeAnchor(p, _) => Some(return_relative_path(base, &p)),
             LinkType::Playpen(p, _) => Some(return_relative_path(base, &p)),
         }
@@ -133,47 +180,35 @@ fn return_relative_path<P: AsRef<Path>>(base: P, relative: P) -> PathBuf {
 }
 
 fn parse_include_path(path: &str) -> LinkType<'static> {
-    let mut parts = path.split(':');
+    let mut parts = path.splitn(4, ':').fuse();
     let path = parts.next().unwrap().into();
 
     let next_element = parts.next();
     let start = if let Some(value) = next_element.and_then(|s| s.parse::<usize>().ok()) {
         // subtract 1 since line numbers usually begin with 1
         Some(value.saturating_sub(1))
+    } else if let Some("") = next_element {
+        None
     } else if let Some(anchor) = next_element {
-        if anchor == "" {
+        return LinkType::IncludeAnchor(path, String::from(anchor));
-            None
-        } else {
-            return LinkType::IncludeAnchor(path, String::from(anchor));
-        }
     } else {
         None
     };
 
     let end = parts.next();
-    let has_end = end.is_some();
+    // If `end` is empty string or any other value that can't be parsed as a usize, treat this
-    let end = end.and_then(|s| s.parse::<usize>().ok());
+    // include as a range with only a start bound. However, if end isn't specified, include only
-    match start {
+    // the single line specified by `start`.
-        Some(start) => match end {
+    let end = end.map(|s| s.parse::<usize>());
-            Some(end) => LinkType::IncludeRange(path, Range { start, end }),
+
-            None => {
+    match (start, end) {
-                if has_end {
+        (Some(start), Some(Ok(end))) => LinkType::IncludeRange(path, LineRange::from(start..end)),
-                    LinkType::IncludeRangeFrom(path, RangeFrom { start })
+        (Some(start), Some(Err(_))) => LinkType::IncludeRange(path, LineRange::from(start..)),
-                } else {
+        (Some(start), None) => LinkType::IncludeRange(path, LineRange::from(start..start + 1)),
-                    LinkType::IncludeRange(
+        (None, Some(Ok(end))) => LinkType::IncludeRange(path, LineRange::from(..end)),
-                        path,
+        (None, None) | (None, Some(Err(_))) => {
-                        Range {
+            LinkType::IncludeRange(path, LineRange::from(RangeFull))
-                            start,
+        }
-                            end: start + 1,
-                        },
-                    )
-                }
-            }
-        },
-        None => match end {
-            Some(end) => LinkType::IncludeRangeTo(path, RangeTo { end }),
-            None => LinkType::IncludeRangeFull(path, RangeFull),
-        },
     }
 }
 
@@ -233,43 +268,6 @@ impl<'a> Link<'a> {
                         )
                     })
             }
-            LinkType::IncludeRangeFrom(ref pat, ref range) => {
-                let target = base.join(pat);
-
-                fs::read_to_string(&target)
-                    .map(|s| take_lines(&s, range.clone()))
-                    .chain_err(|| {
-                        format!(
-                            "Could not read file for link {} ({})",
-                            self.link_text,
-                            target.display(),
-                        )
-                    })
-            }
-            LinkType::IncludeRangeTo(ref pat, ref range) => {
-                let target = base.join(pat);
-
-                fs::read_to_string(&target)
-                    .map(|s| take_lines(&s, *range))
-                    .chain_err(|| {
-                        format!(
-                            "Could not read file for link {} ({})",
-                            self.link_text,
-                            target.display(),
-                        )
-                    })
-            }
-            LinkType::IncludeRangeFull(ref pat, _) => {
-                let target = base.join(pat);
-
-                fs::read_to_string(&target).chain_err(|| {
-                    format!(
-                        "Could not read file for link {} ({})",
-                        self.link_text,
-                        target.display()
-                    )
-                })
-            }
             LinkType::IncludeAnchor(ref pat, ref anchor) => {
                 let target = base.join(pat);
 
@@ -421,7 +419,7 @@ mod tests {
             vec![Link {
                 start_index: 22,
                 end_index: 48,
-                link_type: LinkType::IncludeRange(PathBuf::from("file.rs"), 9..20),
+                link_type: LinkType::IncludeRange(PathBuf::from("file.rs"), LineRange::from(9..20)),
                 link_text: "{{#include file.rs:10:20}}",
             }]
         );
@@ -437,7 +435,7 @@ mod tests {
             vec![Link {
                 start_index: 22,
                 end_index: 45,
-                link_type: LinkType::IncludeRange(PathBuf::from("file.rs"), 9..10),
+                link_type: LinkType::IncludeRange(PathBuf::from("file.rs"), LineRange::from(9..10)),
                 link_text: "{{#include file.rs:10}}",
             }]
         );
@@ -453,7 +451,7 @@ mod tests {
             vec![Link {
                 start_index: 22,
                 end_index: 46,
-                link_type: LinkType::IncludeRangeFrom(PathBuf::from("file.rs"), 9..),
+                link_type: LinkType::IncludeRange(PathBuf::from("file.rs"), LineRange::from(9..)),
                 link_text: "{{#include file.rs:10:}}",
             }]
         );
@@ -469,7 +467,7 @@ mod tests {
             vec![Link {
                 start_index: 22,
                 end_index: 46,
-                link_type: LinkType::IncludeRangeTo(PathBuf::from("file.rs"), ..20),
+                link_type: LinkType::IncludeRange(PathBuf::from("file.rs"), LineRange::from(..20)),
                 link_text: "{{#include file.rs::20}}",
             }]
         );
@@ -485,7 +483,7 @@ mod tests {
             vec![Link {
                 start_index: 22,
                 end_index: 44,
-                link_type: LinkType::IncludeRangeFull(PathBuf::from("file.rs"), ..),
+                link_type: LinkType::IncludeRange(PathBuf::from("file.rs"), LineRange::from(..)),
                 link_text: "{{#include file.rs::}}",
             }]
         );
@@ -501,7 +499,7 @@ mod tests {
             vec![Link {
                 start_index: 22,
                 end_index: 42,
-                link_type: LinkType::IncludeRangeFull(PathBuf::from("file.rs"), ..),
+                link_type: LinkType::IncludeRange(PathBuf::from("file.rs"), LineRange::from(..)),
                 link_text: "{{#include file.rs}}",
             }]
         );
@@ -587,7 +585,7 @@ mod tests {
             Link {
                 start_index: 38,
                 end_index: 58,
-                link_type: LinkType::IncludeRangeFull(PathBuf::from("file.rs"), ..),
+                link_type: LinkType::IncludeRange(PathBuf::from("file.rs"), LineRange::from(..)),
                 link_text: "{{#include file.rs}}",
             }
         );
@@ -614,4 +612,138 @@ mod tests {
         );
     }
 
+    #[test]
+    fn parse_without_colon_includes_all() {
+        let link_type = parse_include_path("arbitrary");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(RangeFull))
+        );
+    }
+
+    #[test]
+    fn parse_with_nothing_after_colon_includes_all() {
+        let link_type = parse_include_path("arbitrary:");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(RangeFull))
+        );
+    }
+
+    #[test]
+    fn parse_with_two_colons_includes_all() {
+        let link_type = parse_include_path("arbitrary::");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(RangeFull))
+        );
+    }
+
+    #[test]
+    fn parse_with_garbage_after_two_colons_includes_all() {
+        let link_type = parse_include_path("arbitrary::NaN");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(RangeFull))
+        );
+    }
+
+    #[test]
+    fn parse_with_one_number_after_colon_only_that_line() {
+        let link_type = parse_include_path("arbitrary:5");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(4..5))
+        );
+    }
+
+    #[test]
+    fn parse_with_one_based_start_becomes_zero_based() {
+        let link_type = parse_include_path("arbitrary:1");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(0..1))
+        );
+    }
+
+    #[test]
+    fn parse_with_zero_based_start_stays_zero_based_but_is_probably_an_error() {
+        let link_type = parse_include_path("arbitrary:0");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(0..1))
+        );
+    }
+
+    #[test]
+    fn parse_start_only_range() {
+        let link_type = parse_include_path("arbitrary:5:");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(4..))
+        );
+    }
+
+    #[test]
+    fn parse_start_with_garbage_interpreted_as_start_only_range() {
+        let link_type = parse_include_path("arbitrary:5:NaN");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(4..))
+        );
+    }
+
+    #[test]
+    fn parse_end_only_range() {
+        let link_type = parse_include_path("arbitrary::5");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(..5))
+        );
+    }
+
+    #[test]
+    fn parse_start_and_end_range() {
+        let link_type = parse_include_path("arbitrary:5:10");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(4..10))
+        );
+    }
+
+    #[test]
+    fn parse_with_negative_interpreted_as_anchor() {
+        let link_type = parse_include_path("arbitrary:-5");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeAnchor(PathBuf::from("arbitrary"), "-5".to_string())
+        );
+    }
+
+    #[test]
+    fn parse_with_floating_point_interpreted_as_anchor() {
+        let link_type = parse_include_path("arbitrary:-5.7");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeAnchor(PathBuf::from("arbitrary"), "-5.7".to_string())
+        );
+    }
+
+    #[test]
+    fn parse_with_anchor_followed_by_colon() {
+        let link_type = parse_include_path("arbitrary:some-anchor:this-gets-ignored");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeAnchor(PathBuf::from("arbitrary"), "some-anchor".to_string())
+        );
+    }
+
+    #[test]
+    fn parse_with_more_than_three_colons_ignores_everything_after_third_colon() {
+        let link_type = parse_include_path("arbitrary:5:10:17:anything:");
+        assert_eq!(
+            link_type,
+            LinkType::IncludeRange(PathBuf::from("arbitrary"), LineRange::from(4..10))
+        );
+    }
 }