update tail tests for more compatibility

tests/coreutils/tail.rs

@@ -2,97 +2,111 @@ use std::{ffi::OsString, path::PathBuf};
 
 use uutils_args::{Arguments, Initial, Options, Value};
 
-#[derive(Arguments)]
-enum DeprecatedArg {
-    #[option("{N}")]
-    Shorthand(Shorthand),
-    #[positional]
-    File(PathBuf),
-}
+// This format is way to specific to implement using a library. Basically, any
+// deviation should be return `None` to indicate that we're not using the
+// this format. If this fails, we fall back on the normal parsing, so errors
+// from this function are not relevant, so we can just return an `Option`.
+// Once this gets into uutils, I highly recommend that we make this format
+// optional at compile time. As the GNU docs explain, it's very error-prone.
+fn parse_deprecated<I>(iter: I) -> Option<Settings>
+where
+    I: IntoIterator + Clone + 'static,
+    I::Item: Into<OsString>,
+{
+    let mut iter = iter.into_iter();
 
-impl Options<DeprecatedArg> for Settings {
-    fn apply(&mut self, arg: DeprecatedArg) {
-        match arg {
-            DeprecatedArg::Shorthand(Shorthand { num, mode, follow }) => {
-                self.number = num;
-                self.mode = mode;
-                self.follow = follow.then_some(FollowMode::Descriptor);
-            }
-            DeprecatedArg::File(file) => {
-                self.inputs.push(file);
-            }
-        }
+    // We don't use it, but the first argument is the binary name.
+    let _ = iter.next();
+
+    let shorthand = iter.next()?;
+    let input = iter.next()?;
+
+    // We can only have a maximum of 2 arguments in this format
+    // The error doesn't really matter because we'll ignore any errors
+    // from this format.
+    if iter.next().is_some() {
+        return None;
     }
-}
 
-struct Shorthand {
-    num: SigNum,
-    mode: Mode,
-    follow: bool,
-}
+    // Parse the shorthand by turning it into a String (via OsString)
+    // The format we're parsing is `{+/-}[NUM][bcl][f]`.
+    let os_string = shorthand.into();
 
-// This is not technically 100% compatible with GNU, because the shorthand can
-// appear as any argument, not just the first.
-impl Value for Shorthand {
-    fn from_value(value: &std::ffi::OsStr) -> uutils_args::ValueResult<Self> {
-        let s = String::from_value(value)?;
+    // The part of the string that is not parsed yet
+    let mut rest = os_string.to_str()?;
 
-        let mut rest: &str = &s;
-
-        let sig = if let Some(r) = rest.strip_prefix('-') {
-            rest = r;
-            SigNum::Negative
-        } else if let Some(r) = rest.strip_prefix('+') {
-            rest = r;
-            SigNum::Positive
-        } else {
-            return Err("Invalid shorthand".into());
-        };
-
-        // Find and parse the number part of the string
-        let end_num = rest
-            .find(|c: char| !c.is_ascii_digit())
-            .unwrap_or(rest.len());
-        let num = rest[..end_num].parse().unwrap_or(10);
-        rest = &rest[end_num..];
-
-        let mode = if let Some(r) = rest.strip_prefix('l') {
-            rest = r;
-            Mode::Lines
-        } else if let Some(r) = rest.strip_prefix('c') {
-            rest = r;
-            Mode::Bytes
-        } else if let Some(r) = rest.strip_prefix('b') {
-            rest = r;
-            Mode::Blocks
-        } else {
-            Mode::Lines
-        };
-
-        let follow = if let Some(r) = rest.strip_prefix('f') {
-            rest = r;
-            true
-        } else {
-            false
-        };
-
-        if !rest.is_empty() {
-            return Err("Invalid shorthand!".into());
-        }
-
-        Ok(Self {
-            num: sig(num),
-            mode,
-            follow,
-        })
+    // Corner case: If it's just `-` then it needs to be parsed like
+    // the nondeprecated syntax, because `-` represents standard input.
+    // Curiously, GNU parses `tail + a.txt` as the deprecated syntax.
+    if rest == "-" {
+        return None;
     }
+
+    // Corner case: `tail -c 10` is ambiguous and should be interpreted as
+    // `tail -c10 -`, not as `tail -c10 10`. All other things in this syntax
+    // do not create problems. For example, `tail -f a` has the same effect in
+    // this syntax and normal parsing.
+    if rest == "-c" {
+        return None;
+    }
+
+    // Parse the sign
+    let sig = if let Some(r) = rest.strip_prefix('-') {
+        rest = r;
+        SigNum::Negative
+    } else if let Some(r) = rest.strip_prefix('+') {
+        rest = r;
+        SigNum::Positive
+    } else {
+        return None;
+    };
+
+    // Find and parse the number part of the string
+    let end_num = rest
+        .find(|c: char| !c.is_ascii_digit())
+        .unwrap_or(rest.len());
+    let num = rest[..end_num].parse().unwrap_or(10);
+    rest = &rest[end_num..];
+
+    // Parse the mode, one of `b`, `c`, 'l`.
+    let mode = if let Some(r) = rest.strip_prefix('l') {
+        rest = r;
+        Mode::Lines
+    } else if let Some(r) = rest.strip_prefix('c') {
+        rest = r;
+        Mode::Bytes
+    } else if let Some(r) = rest.strip_prefix('b') {
+        rest = r;
+        Mode::Blocks
+    } else {
+        Mode::Lines
+    };
+
+    // Parse the `f`
+    let follow = if let Some(r) = rest.strip_prefix('f') {
+        rest = r;
+        Some(FollowMode::Descriptor)
+    } else {
+        None
+    };
+
+    if !rest.is_empty() {
+        return None;
+    }
+
+    Some(Settings {
+        number: sig(num),
+        mode,
+        follow,
+        inputs: vec![input.into().into()],
+        ..Settings::initial()
+    })
 }
 
 #[derive(Arguments)]
 enum Arg {
-    // TODO: Bytes and Lines should take a `SigNum`
     #[option("-c NUM", "--bytes=NUM")]
-    Bytes(u64),
+    Bytes(SigNum),
 
     #[option("-f", "--follow[=HOW]", default=FollowMode::Descriptor)]
     Follow(FollowMode),
@@ -104,7 +118,7 @@ enum Arg {
     MaxUnchangedStats(u32),
 
     #[option("-n NUM", "--lines=NUM")]
-    Lines(u64),
+    Lines(SigNum),
 
     #[option("--pid=PID")]
     Pid(u64),
@@ -138,6 +152,45 @@ enum SigNum {
     Negative(u64),
 }
 
+impl Value for SigNum {
+    fn from_value(value: &std::ffi::OsStr) -> uutils_args::ValueResult<Self> {
+        let s = String::from_value(value)?;
+        let mut rest: &str = &s;
+
+        let sign = if let Some(r) = s.strip_prefix('+') {
+            rest = r;
+            Self::Positive
+        } else if let Some(r) = s.strip_prefix('-') {
+            rest = r;
+            Self::Negative
+        } else {
+            Self::Negative
+        };
+
+        // Get the number from the front of the string
+        let end_num = rest
+            .find(|c: char| !c.is_ascii_digit())
+            .unwrap_or(rest.len());
+        let num = rest[..end_num].parse().unwrap_or(1);
+        rest = &rest[end_num..];
+
+        // Determine the multiplier
+        let multiplier = match rest {
+            "" => 1,
+            "b" => 512,                 // blocks
+            "K" => 1024,                // kibibytes
+            "M" => 1024 * 1024,         // mebibytes
+            "G" => 1024 * 1024 * 1024,  // gibibytes
+            "KB" => 1000,               // kilobytes
+            "MB" => 1000 * 1000,        // megabytes
+            "GB" => 1000 * 1000 * 1000, // gigabytes
+            _ => return Err(format!("Invalid number of lines: {s}").into()),
+        };
+
+        Ok(sign(num * multiplier))
+    }
+}
+
 impl Default for SigNum {
     fn default() -> Self {
         Self::Negative(10)
@@ -166,7 +219,7 @@ struct Settings {
     max_unchanged_stats: u32,
     mode: Mode,
     number: SigNum,
-    // Should be a dedicated PID type
+    // TODO: Should be a dedicated PID type
     pid: u64,
     retry: bool,
     sleep_sec: u64,
@@ -181,11 +234,11 @@ impl Options<Arg> for Settings {
         match arg {
             Arg::Bytes(n) => {
                 self.mode = Mode::Bytes;
-                self.number = SigNum::Negative(n);
+                self.number = n;
             }
             Arg::Lines(n) => {
                 self.mode = Mode::Lines;
-                self.number = SigNum::Negative(n);
+                self.number = n;
             }
             Arg::Follow(mode) => self.follow = Some(mode),
             Arg::FollowRetry => {
@@ -210,9 +263,12 @@ where
     I: IntoIterator + Clone + 'static,
     I::Item: Into<OsString>,
 {
-    <Settings as Options<DeprecatedArg>>::try_parse(iter.clone())
-        .or_else(|_| <Settings as Options<Arg>>::try_parse(iter))
+    match parse_deprecated(iter.clone()) {
+        Some(s) => Ok(s),
+        None => Settings::try_parse(iter),
+    }
 }
+
 #[test]
 fn shorthand() {
     let s = parse_tail(["tail", "-20", "somefile"]).unwrap();
@@ -229,4 +285,81 @@ fn shorthand() {
     assert_eq!(s.number, SigNum::Negative(100));
     assert_eq!(s.mode, Mode::Bytes);
     assert_eq!(s.follow, Some(FollowMode::Descriptor));
+
+    // Corner case where the shorthand does not apply
+    let s = parse_tail(["tail", "-c", "42"]).unwrap();
+    assert_eq!(s.number, SigNum::Negative(42));
+    assert_eq!(s.mode, Mode::Bytes);
+    assert_eq!(s.inputs, Vec::<PathBuf>::new());
+}
+
+#[test]
+fn standard_input() {
+    let s = parse_tail(["tail", "-"]).unwrap();
+    assert_eq!(s.inputs, vec![PathBuf::from("-")])
+}
+
+#[test]
+fn normal_format() {
+    let s = parse_tail(["tail", "-c", "20", "somefile"]).unwrap();
+    assert_eq!(s.number, SigNum::Negative(20));
+    assert_eq!(s.mode, Mode::Bytes);
+}
+
+#[test]
+fn signum() {
+    let s = parse_tail(["tail", "-n", "20"]).unwrap();
+    assert_eq!(s.number, SigNum::Negative(20));
+    let s = parse_tail(["tail", "-n", "-20"]).unwrap();
+    assert_eq!(s.number, SigNum::Negative(20));
+    let s = parse_tail(["tail", "-n", "+20"]).unwrap();
+    assert_eq!(s.number, SigNum::Positive(20));
+
+    let s = parse_tail(["tail", "-n", "20b"]).unwrap();
+    assert_eq!(s.number, SigNum::Negative(20 * 512));
+    let s = parse_tail(["tail", "-n", "+20b"]).unwrap();
+    assert_eq!(s.number, SigNum::Positive(20 * 512));
+
+    let s = parse_tail(["tail", "-n", "b"]).unwrap();
+    assert_eq!(s.number, SigNum::Negative(512));
+    let s = parse_tail(["tail", "-n", "+b"]).unwrap();
+    assert_eq!(s.number, SigNum::Positive(512));
+
+    assert!(parse_tail(["tail", "-n", "20invalid_suffix"]).is_err());
+}
+
+#[test]
+fn followmode() {
+    // Sanity check: should be None initially
+    let s = parse_tail(["tail"]).unwrap();
+    assert_eq!(s.follow, None);
+
+    let s = parse_tail(["tail", "--follow"]).unwrap();
+    assert_eq!(s.follow, Some(FollowMode::Descriptor));
+
+    let s = parse_tail(["tail", "-f"]).unwrap();
+    assert_eq!(s.follow, Some(FollowMode::Descriptor));
+
+    let s = parse_tail(["tail", "--follow=descriptor"]).unwrap();
+    assert_eq!(s.follow, Some(FollowMode::Descriptor));
+
+    let s = parse_tail(["tail", "--follow=des"]).unwrap();
+    assert_eq!(s.follow, Some(FollowMode::Descriptor));
+
+    let s = parse_tail(["tail", "--follow=d"]).unwrap();
+    assert_eq!(s.follow, Some(FollowMode::Descriptor));
+
+    let s = parse_tail(["tail", "--follow=name"]).unwrap();
+    assert_eq!(s.follow, Some(FollowMode::Name));
+
+    let s = parse_tail(["tail", "--follow=na"]).unwrap();
+    assert_eq!(s.follow, Some(FollowMode::Name));
+
+    let s = parse_tail(["tail", "--follow=n"]).unwrap();
+    assert_eq!(s.follow, Some(FollowMode::Name));
+
+    assert!(parse_tail(["tail", "--follow="]).is_err());
+
+    let s = parse_tail(["tail", "-F"]).unwrap();
+    assert_eq!(s.follow, Some(FollowMode::Name));
 }