Repository: sharkdp/bat
Pull request #368 (2018-10-19 22:13:34)

Allow for multiple line ranges
See #23

ErichDonGubler: Total bikeshed you can ignore: when you first said "multiple line ranges", I originally read it as "*more than one* line range" -- but I know better, since I've been involved with your development of this idea! Maybe "multi-line ranges" is less ambiguous?

Patch:
diff --git a/src/app.rs b/src/app.rs
index 30724b21..06a0051f 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -17,7 +17,7 @@ use assets::BAT_THEME_DEFAULT;
 use config::{get_args_from_config_file, get_args_from_env_var};
 use errors::*;
 use inputfile::InputFile;
-use line_range::LineRange;
+use line_range::{LineRange, LineRanges};
 use style::{OutputComponent, OutputComponents, OutputWrap};
 use syntax_mapping::SyntaxMapping;
 use util::transpose;
@@ -62,8 +62,8 @@ pub struct Config<'a> {
     /// Pager or STDOUT
     pub paging_mode: PagingMode,
 
-    /// The range lines that should be printed, if specified
-    pub line_range: Option<LineRange>,
+    /// Specifies the lines that should be printed
+    pub line_ranges: LineRanges,
 
     /// The syntax highlighting theme
     pub theme: String,
@@ -218,7 +218,14 @@ impl App {
                 .map(String::from)
                 .or_else(|| env::var("BAT_THEME").ok())
                 .unwrap_or(String::from(BAT_THEME_DEFAULT)),
-            line_range: transpose(self.matches.value_of("line-range").map(LineRange::from))?,
+            line_ranges: LineRanges::from(
+                transpose(
+                    self.matches
+                        .values_of("line-range")
+                        .map(|vs| vs.map(LineRange::from).collect()),
+                )?
+                .unwrap_or(vec![]),
+            ),
             output_components,
             syntax_mapping,
         })
diff --git a/src/clap_app.rs b/src/clap_app.rs
index b22f4f03..25ed5517 100644
--- a/src/clap_app.rs
+++ b/src/clap_app.rs
@@ -146,8 +146,9 @@ pub fn build_app(interactive_output: bool) -> ClapApp<'static, 'static> {
         .arg(
             Arg::with_name("line-range")
                 .long("line-range")
-                .overrides_with("line-range")
+                .multiple(true)
                 .takes_value(true)
+                .number_of_values(1)
                 .value_name("N:M")
                 .help("Only print the lines from N to M.")
                 .long_help(
diff --git a/src/controller.rs b/src/controller.rs
index d9a53786..bf8cef13 100644
--- a/src/controller.rs
+++ b/src/controller.rs
@@ -4,7 +4,7 @@ use app::Config;
 use assets::HighlightingAssets;
 use errors::*;
 use inputfile::{InputFile, InputFileReader};
-use line_range::LineRange;
+use line_range::{LineRanges, RangeCheckResult};
 use output::OutputType;
 use printer::{InteractivePrinter, Printer, SimplePrinter};
 
@@ -64,7 +64,7 @@ impl<'b> Controller<'b> {
         input_file: InputFile<'a>,
     ) -> Result<()> {
         printer.print_header(writer, input_file)?;
-        self.print_file_ranges(printer, writer, reader, &self.config.line_range)?;
+        self.print_file_ranges(printer, writer, reader, &self.config.line_ranges)?;
         printer.print_footer(writer)?;
 
         Ok(())
@@ -75,29 +75,25 @@ impl<'b> Controller<'b> {
         printer: &mut P,
         writer: &mut Write,
         mut reader: InputFileReader,
-        line_ranges: &Option<LineRange>,
+        line_ranges: &LineRanges,
     ) -> Result<()> {
         let mut line_buffer = Vec::new();
 
         let mut line_number: usize = 1;
 
         while reader.read_line(&mut line_buffer)? {
-            match line_ranges {
-                &Some(ref range) => {
-                    if line_number < range.lower {
-                        // Call the printer in case we need to call the syntax highlighter
-                        // for this line. However, set `out_of_range` to `true`.
-                        printer.print_line(true, writer, line_number, &line_buffer)?;
-                    } else if line_number > range.upper {
-                        // no more lines in range, exit early
-                        break;
-                    } else {
-                        printer.print_line(false, writer, line_number, &line_buffer)?;
-                    }
+            match line_ranges.check(line_number) {
+                RangeCheckResult::OutsideRange => {
+                    // Call the printer in case we need to call the syntax highlighter
+                    // for this line. However, set `out_of_range` to `true`.
+                    printer.print_line(true, writer, line_number, &line_buffer)?;
                 }
-                &None => {
+                RangeCheckResult::InRange => {
                     printer.print_line(false, writer, line_number, &line_buffer)?;
                 }
+                RangeCheckResult::AfterLastRange => {
+                    break;
+                }
             }
 
             line_number += 1;
diff --git a/src/line_range.rs b/src/line_range.rs
index 9f8d538b..f1346ffb 100644
--- a/src/line_range.rs
+++ b/src/line_range.rs
@@ -38,6 +38,10 @@ impl LineRange {
 
         Err("expected single ':' character".into())
     }
+
+    pub fn is_inside(&self, line: usize) -> bool {
+        line >= self.lower && line <= self.upper
+    }
 }
 
 #[test]
@@ -72,3 +76,105 @@ fn test_parse_fail() {
     let range = LineRange::from("40");
     assert!(range.is_err());
 }
+
+#[derive(Copy, Clone, Debug, PartialEq)]
+pub enum RangeCheckResult {
+    // Within one of the given ranges
+    InRange,
+
+    // Before the first range or within two ranges
+    OutsideRange,
+
+    // Line number is outside of all ranges and larger than the last range.
+    AfterLastRange,
+}
+
+#[derive(Clone)]
+pub struct LineRanges {
+    ranges: Vec<LineRange>,
+    largest_upper_bound: usize,
+}
+
+impl LineRanges {
+    pub fn from(ranges: Vec<LineRange>) -> LineRanges {
+        let largest_upper_bound = ranges
+            .iter()
+            .map(|r| r.upper)
+            .max()
+            .unwrap_or(usize::max_value());
+        LineRanges {
+            ranges,
+            largest_upper_bound,
+        }
+    }
+
+    pub fn check(&self, line: usize) -> RangeCheckResult {
+        if self.ranges.is_empty() {
+            RangeCheckResult::InRange
+        } else {
+            if self.ranges.iter().any(|r| r.is_inside(line)) {
+                RangeCheckResult::InRange
+            } else {
+                if line < self.largest_upper_bound {
+                    RangeCheckResult::OutsideRange
+                } else {
+                    RangeCheckResult::AfterLastRange
+                }
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+fn ranges(rs: &[&str]) -> LineRanges {
+    LineRanges::from(rs.iter().map(|r| LineRange::from(r).unwrap()).collect())
+}
+
+#[test]
+fn test_ranges_simple() {
+    let ranges = ranges(&["3:8"]);
+
+    assert_eq!(RangeCheckResult::OutsideRange, ranges.check(2));
+    assert_eq!(RangeCheckResult::InRange, ranges.check(5));
+    assert_eq!(RangeCheckResult::AfterLastRange, ranges.check(9));
+}
+
+#[test]
+fn test_ranges_advanced() {
+    let ranges = ranges(&["3:8", "11:20", "25:30"]);
+
+    assert_eq!(RangeCheckResult::OutsideRange, ranges.check(2));
+    assert_eq!(RangeCheckResult::InRange, ranges.check(5));
+    assert_eq!(RangeCheckResult::OutsideRange, ranges.check(9));
+    assert_eq!(RangeCheckResult::InRange, ranges.check(11));
+    assert_eq!(RangeCheckResult::OutsideRange, ranges.check(22));
+    assert_eq!(RangeCheckResult::InRange, ranges.check(28));
+    assert_eq!(RangeCheckResult::AfterLastRange, ranges.check(31));
+}
+
+#[test]
+fn test_ranges_open_low() {
+    let ranges = ranges(&["3:8", ":5"]);
+
+    assert_eq!(RangeCheckResult::InRange, ranges.check(1));
+    assert_eq!(RangeCheckResult::InRange, ranges.check(3));
+    assert_eq!(RangeCheckResult::InRange, ranges.check(7));
+    assert_eq!(RangeCheckResult::AfterLastRange, ranges.check(9));
+}
+
+#[test]
+fn test_ranges_open_high() {
+    let ranges = ranges(&["3:", "2:5"]);
+
+    assert_eq!(RangeCheckResult::OutsideRange, ranges.check(1));
+    assert_eq!(RangeCheckResult::InRange, ranges.check(3));
+    assert_eq!(RangeCheckResult::InRange, ranges.check(5));
+    assert_eq!(RangeCheckResult::InRange, ranges.check(9));
+}
+
+#[test]
+fn test_ranges_empty() {
+    let ranges = ranges(&[]);
+
+    assert_eq!(RangeCheckResult::InRange, ranges.check(1));
+}


Test patch:
diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index 4edc072e..28f30ddd 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -96,6 +96,17 @@ fn line_range_last_3() {
         .stdout("line 2\nline 3\nline 4\n");
 }
 
+#[test]
+fn line_range_multiple() {
+    bat()
+        .arg("multiline.txt")
+        .arg("--line-range=1:2")
+        .arg("--line-range=4:4")
+        .assert()
+        .success()
+        .stdout("line 1\nline 2\nline 4\n");
+}
+
 #[test]
 fn tabs_numbers() {
     bat()