tail: improve GNU compatibility

src/uu/tail/src/args.rs

@@ -64,7 +64,12 @@ impl FilterMode {
         let mode = if let Some(arg) = matches.get_one::<String>(options::BYTES) {
             match parse_num(arg) {
                 Ok(signum) => Self::Bytes(signum),
-                Err(e) => return Err(UUsageError::new(1, format!("invalid number of bytes: {e}"))),
+                Err(e) => {
+                    return Err(USimpleError::new(
+                        1,
+                        format!("invalid number of bytes: {e}"),
+                    ))
+                }
             }
         } else if let Some(arg) = matches.get_one::<String>(options::LINES) {
             match parse_num(arg) {
@@ -72,7 +77,12 @@ impl FilterMode {
                     let delimiter = if zero_term { 0 } else { b'\n' };
                     Self::Lines(signum, delimiter)
                 }
-                Err(e) => return Err(UUsageError::new(1, format!("invalid number of lines: {e}"))),
+                Err(e) => {
+                    return Err(USimpleError::new(
+                        1,
+                        format!("invalid number of lines: {e}"),
+                    ))
+                }
             }
         } else if zero_term {
             Self::default_zero()
@@ -307,14 +317,19 @@ pub fn arg_iterate<'a>(
         if let Some(s) = second.to_str() {
             match parse::parse_obsolete(s) {
                 Some(Ok(iter)) => Ok(Box::new(vec![first].into_iter().chain(iter).chain(args))),
-                Some(Err(e)) => Err(UUsageError::new(
+                Some(Err(e)) => Err(USimpleError::new(
                     1,
                     match e {
-                        parse::ParseError::Syntax => format!("bad argument format: {}", s.quote()),
                         parse::ParseError::Overflow => format!(
                             "invalid argument: {} Value too large for defined datatype",
                             s.quote()
                         ),
+                        parse::ParseError::Context => {
+                            format!(
+                                "option used in invalid context -- {}",
+                                s.chars().nth(1).unwrap_or_default()
+                            )
+                        }
                     },
                 )),
                 None => Ok(Box::new(vec![first, second].into_iter().chain(args))),

src/uu/tail/src/parse.rs

@@ -7,92 +7,67 @@ use std::ffi::OsString;
 
 #[derive(PartialEq, Eq, Debug)]
 pub enum ParseError {
-    Syntax,
     Overflow,
+    Context,
 }
 /// Parses obsolete syntax
-/// tail -NUM\[kmzv\] // spell-checker:disable-line
+/// tail -\[NUM\]\[bl\]\[f\] and tail +\[NUM\]\[bcl\]\[f\] // spell-checker:disable-line
 pub fn parse_obsolete(src: &str) -> Option<Result<impl Iterator<Item = OsString>, ParseError>> {
-    let mut chars = src.char_indices();
+    let mut chars = src.chars();
-    if let Some((_, '-')) = chars.next() {
+    let sign = chars.next()?;
-        let mut num_end = 0usize;
+    if sign != '+' && sign != '-' {
-        let mut has_num = false;
+        return None;
-        let mut last_char = 0 as char;
+    }
-        for (n, c) in &mut chars {
+
-            if c.is_ascii_digit() {
+    let numbers: String = chars.clone().take_while(|&c| c.is_ascii_digit()).collect();
-                has_num = true;
+    let has_num = !numbers.is_empty();
-                num_end = n;
+    let num: usize = if has_num {
-            } else {
+        if let Ok(num) = numbers.parse() {
-                last_char = c;
+            num
-                break;
-            }
-        }
-        if has_num {
-            match src[1..=num_end].parse::<usize>() {
-                Ok(num) => {
-                    let mut quiet = false;
-                    let mut verbose = false;
-                    let mut zero_terminated = false;
-                    let mut multiplier = None;
-                    let mut c = last_char;
-                    loop {
-                        // not that here, we only match lower case 'k', 'c', and 'm'
-                        match c {
-                            // we want to preserve order
-                            // this also saves us 1 heap allocation
-                            'q' => {
-                                quiet = true;
-                                verbose = false;
-                            }
-                            'v' => {
-                                verbose = true;
-                                quiet = false;
-                            }
-                            'z' => zero_terminated = true,
-                            'c' => multiplier = Some(1),
-                            'b' => multiplier = Some(512),
-                            'k' => multiplier = Some(1024),
-                            'm' => multiplier = Some(1024 * 1024),
-                            '\0' => {}
-                            _ => return Some(Err(ParseError::Syntax)),
-                        }
-                        if let Some((_, next)) = chars.next() {
-                            c = next;
-                        } else {
-                            break;
-                        }
-                    }
-                    let mut options = Vec::new();
-                    if quiet {
-                        options.push(OsString::from("-q"));
-                    }
-                    if verbose {
-                        options.push(OsString::from("-v"));
-                    }
-                    if zero_terminated {
-                        options.push(OsString::from("-z"));
-                    }
-                    if let Some(n) = multiplier {
-                        options.push(OsString::from("-c"));
-                        let num = match num.checked_mul(n) {
-                            Some(n) => n,
-                            None => return Some(Err(ParseError::Overflow)),
-                        };
-                        options.push(OsString::from(format!("{num}")));
-                    } else {
-                        options.push(OsString::from("-n"));
-                        options.push(OsString::from(format!("{num}")));
-                    }
-                    Some(Ok(options.into_iter()))
-                }
-                Err(_) => Some(Err(ParseError::Overflow)),
-            }
         } else {
-            None
+            return Some(Err(ParseError::Overflow));
         }
     } else {
-        None
+        10
+    };
+
+    let mut follow = false;
+    let mut mode = None;
+    let mut first_char = true;
+    for char in chars.skip_while(|&c| c.is_ascii_digit()) {
+        if sign == '-' && char == 'c' && !has_num {
+            // special case: -c should be handled by clap (is ambiguous)
+            return None;
+        } else if char == 'f' {
+            follow = true;
+        } else if first_char && (char == 'b' || char == 'c' || char == 'l') {
+            mode = Some(char);
+        } else if has_num && sign == '-' {
+            return Some(Err(ParseError::Context));
+        } else {
+            return None;
+        }
+        first_char = false;
     }
+
+    let mut options = Vec::new();
+    if follow {
+        options.push(OsString::from("-f"));
+    }
+    let mode = mode.unwrap_or('l');
+    if mode == 'b' || mode == 'c' {
+        options.push(OsString::from("-c"));
+        let n = if mode == 'b' { 512 } else { 1 };
+        let num = match num.checked_mul(n) {
+            Some(n) => n,
+            None => return Some(Err(ParseError::Overflow)),
+        };
+        options.push(OsString::from(format!("{sign}{num}")));
+    } else {
+        options.push(OsString::from("-n"));
+        options.push(OsString::from(format!("{sign}{num}")));
+    }
+    Some(Ok(options.into_iter()))
 }
 
 #[cfg(test)]
@@ -113,40 +88,35 @@ mod tests {
     }
     #[test]
     fn test_parse_numbers_obsolete() {
-        assert_eq!(obsolete("-5"), obsolete_result(&["-n", "5"]));
+        assert_eq!(obsolete("+2c"), obsolete_result(&["-c", "+2"]));
-        assert_eq!(obsolete("-100"), obsolete_result(&["-n", "100"]));
+        assert_eq!(obsolete("-5"), obsolete_result(&["-n", "-5"]));
-        assert_eq!(obsolete("-5m"), obsolete_result(&["-c", "5242880"]));
+        assert_eq!(obsolete("-100"), obsolete_result(&["-n", "-100"]));
-        assert_eq!(obsolete("-1k"), obsolete_result(&["-c", "1024"]));
+        assert_eq!(obsolete("-2b"), obsolete_result(&["-c", "-1024"]));
-        assert_eq!(obsolete("-2b"), obsolete_result(&["-c", "1024"]));
-        assert_eq!(obsolete("-1mmk"), obsolete_result(&["-c", "1024"]));
-        assert_eq!(obsolete("-1vz"), obsolete_result(&["-v", "-z", "-n", "1"]));
-        assert_eq!(
-            obsolete("-1vzqvq"), // spell-checker:disable-line
-            obsolete_result(&["-q", "-z", "-n", "1"])
-        );
-        assert_eq!(obsolete("-1vzc"), obsolete_result(&["-v", "-z", "-c", "1"]));
-        assert_eq!(
-            obsolete("-105kzm"),
-            obsolete_result(&["-z", "-c", "110100480"])
-        );
     }
     #[test]
     fn test_parse_errors_obsolete() {
-        assert_eq!(obsolete("-5n"), Some(Err(ParseError::Syntax)));
+        assert_eq!(obsolete("-5n"), Some(Err(ParseError::Context)));
-        assert_eq!(obsolete("-5c5"), Some(Err(ParseError::Syntax)));
+        assert_eq!(obsolete("-5c5"), Some(Err(ParseError::Context)));
+        assert_eq!(obsolete("-1vzc"), Some(Err(ParseError::Context)));
+        assert_eq!(obsolete("-5m"), Some(Err(ParseError::Context)));
+        assert_eq!(obsolete("-1k"), Some(Err(ParseError::Context)));
+        assert_eq!(obsolete("-1mmk"), Some(Err(ParseError::Context)));
+        assert_eq!(obsolete("-105kzm"), Some(Err(ParseError::Context)));
+        assert_eq!(obsolete("-1vz"), Some(Err(ParseError::Context)));
+        assert_eq!(
+            obsolete("-1vzqvq"), // spell-checker:disable-line
+            Some(Err(ParseError::Context))
+        );
     }
     #[test]
     fn test_parse_obsolete_no_match() {
         assert_eq!(obsolete("-k"), None);
         assert_eq!(obsolete("asd"), None);
+        assert_eq!(obsolete("-cc"), None);
     }
     #[test]
     #[cfg(target_pointer_width = "64")]
     fn test_parse_obsolete_overflow_x64() {
-        assert_eq!(
-            obsolete("-1000000000000000m"),
-            Some(Err(ParseError::Overflow))
-        );
         assert_eq!(
             obsolete("-10000000000000000000000"),
             Some(Err(ParseError::Overflow))
@@ -156,6 +126,5 @@ mod tests {
     #[cfg(target_pointer_width = "32")]
     fn test_parse_obsolete_overflow_x32() {
         assert_eq!(obsolete("-42949672960"), Some(Err(ParseError::Overflow)));
-        assert_eq!(obsolete("-42949672k"), Some(Err(ParseError::Overflow)));
     }
 }

tests/by-util/test_tail.rs

@@ -4475,3 +4475,239 @@ fn test_args_sleep_interval_when_illegal_argument_then_usage_error(#[case] sleep
         .usage_error(format!("invalid number of seconds: '{sleep_interval}'"))
         .code_is(1);
 }
+
+#[test]
+fn test_gnu_args_plus_c() {
+    let scene = TestScenario::new(util_name!());
+
+    // obs-plus-c1
+    scene
+        .ucmd()
+        .arg("+2c")
+        .pipe_in("abcd")
+        .succeeds()
+        .stdout_only("bcd");
+    // obs-plus-c2
+    scene
+        .ucmd()
+        .arg("+8c")
+        .pipe_in("abcd")
+        .succeeds()
+        .stdout_only("");
+    // obs-plus-x1: same as +10c
+    scene
+        .ucmd()
+        .arg("+c")
+        .pipe_in(format!("x{}z", "y".repeat(10)))
+        .succeeds()
+        .stdout_only("yyz");
+}
+
+#[test]
+fn test_gnu_args_c() {
+    let scene = TestScenario::new(util_name!());
+
+    // obs-c3
+    scene
+        .ucmd()
+        .arg("-1c")
+        .pipe_in("abcd")
+        .succeeds()
+        .stdout_only("d");
+    // obs-c4
+    scene
+        .ucmd()
+        .arg("-9c")
+        .pipe_in("abcd")
+        .succeeds()
+        .stdout_only("abcd");
+    // obs-c5
+    scene
+        .ucmd()
+        .arg("-12c")
+        .pipe_in(format!("x{}z", "y".repeat(12)))
+        .succeeds()
+        .stdout_only(&format!("{}z", "y".repeat(11)));
+}
+
+#[test]
+fn test_gnu_args_l() {
+    let scene = TestScenario::new(util_name!());
+
+    // obs-l1
+    scene
+        .ucmd()
+        .arg("-1l")
+        .pipe_in("x")
+        .succeeds()
+        .stdout_only("x");
+    // obs-l2
+    scene
+        .ucmd()
+        .arg("-1l")
+        .pipe_in("x\ny\n")
+        .succeeds()
+        .stdout_only("y\n");
+    // obs-l3
+    scene
+        .ucmd()
+        .arg("-1l")
+        .pipe_in("x\ny")
+        .succeeds()
+        .stdout_only("y");
+    // obs-l: same as -10l
+    scene
+        .ucmd()
+        .arg("-l")
+        .pipe_in(format!("x{}z", "y\n".repeat(10)))
+        .succeeds()
+        .stdout_only(&format!("{}z", "y\n".repeat(9)));
+}
+
+#[test]
+fn test_gnu_args_plus_l() {
+    let scene = TestScenario::new(util_name!());
+
+    // obs-plus-l4
+    scene
+        .ucmd()
+        .arg("+1l")
+        .pipe_in("x\ny\n")
+        .succeeds()
+        .stdout_only("x\ny\n");
+    // ops-plus-l5
+    scene
+        .ucmd()
+        .arg("+2l")
+        .pipe_in("x\ny\n")
+        .succeeds()
+        .stdout_only("y\n");
+    // obs-plus-x2: same as +10l
+    scene
+        .ucmd()
+        .arg("+l")
+        .pipe_in(format!("x\n{}z", "y\n".repeat(10)))
+        .succeeds()
+        .stdout_only("y\ny\nz");
+}
+
+#[test]
+fn test_gnu_args_number() {
+    let scene = TestScenario::new(util_name!());
+
+    // obs-1
+    scene
+        .ucmd()
+        .arg("-1")
+        .pipe_in("x")
+        .succeeds()
+        .stdout_only("x");
+    // obs-2
+    scene
+        .ucmd()
+        .arg("-1")
+        .pipe_in("x\ny\n")
+        .succeeds()
+        .stdout_only("y\n");
+    // obs-3
+    scene
+        .ucmd()
+        .arg("-1")
+        .pipe_in("x\ny")
+        .succeeds()
+        .stdout_only("y");
+}
+
+#[test]
+fn test_gnu_args_plus_number() {
+    let scene = TestScenario::new(util_name!());
+
+    // obs-plus-4
+    scene
+        .ucmd()
+        .arg("+1")
+        .pipe_in("x\ny\n")
+        .succeeds()
+        .stdout_only("x\ny\n");
+    // ops-plus-5
+    scene
+        .ucmd()
+        .arg("+2")
+        .pipe_in("x\ny\n")
+        .succeeds()
+        .stdout_only("y\n");
+}
+
+#[test]
+fn test_gnu_args_b() {
+    let scene = TestScenario::new(util_name!());
+
+    // obs-b
+    scene
+        .ucmd()
+        .arg("-b")
+        .pipe_in("x\n".repeat(512 * 10 / 2 + 1))
+        .succeeds()
+        .stdout_only(&"x\n".repeat(512 * 10 / 2));
+}
+
+#[test]
+fn test_gnu_args_err() {
+    let scene = TestScenario::new(util_name!());
+
+    // err-1
+    scene
+        .ucmd()
+        .arg("+cl")
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: cannot open '+cl' for reading: No such file or directory\n")
+        .code_is(1);
+    // err-2
+    scene
+        .ucmd()
+        .arg("-cl")
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: invalid number of bytes: 'l'\n")
+        .code_is(1);
+    // err-3
+    scene
+        .ucmd()
+        .arg("+2cz")
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: cannot open '+2cz' for reading: No such file or directory\n")
+        .code_is(1);
+    // err-4
+    scene
+        .ucmd()
+        .arg("-2cX")
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: option used in invalid context -- 2\n")
+        .code_is(1);
+    // err-5
+    scene
+        .ucmd()
+        .arg("-c99999999999999999999")
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: invalid number of bytes: '99999999999999999999'\n")
+        .code_is(1);
+    // err-6
+    scene
+        .ucmd()
+        .arg("-c --")
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: invalid number of bytes: '-'\n")
+        .code_is(1);
+    scene
+        .ucmd()
+        .arg("-5cz")
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: option used in invalid context -- 5\n")
+        .code_is(1);
+}