tail: simplify obsolete parsing

src/uu/tail/src/args.rs

@@ -348,39 +348,33 @@ impl Settings {
 
         VerificationResult::Ok
     }
-
-    pub fn is_default(&self) -> bool {
-        let default = Self::default();
-        self.max_unchanged_stats == default.max_unchanged_stats
-            && self.sleep_sec == default.sleep_sec
-            && self.follow == default.follow
-            && self.mode == default.mode
-            && self.pid == default.pid
-            && self.retry == default.retry
-            && self.use_polling == default.use_polling
-            && (self.verbose == default.verbose || self.inputs.len() > 1)
-            && self.presume_input_pipe == default.presume_input_pipe
-    }
 }
 
-pub fn parse_obsolete(args: &str) -> UResult<Option<parse::ObsoleteArgs>> {
-    match parse::parse_obsolete(args) {
-        Some(Ok(args)) => Ok(Some(args)),
+pub fn parse_obsolete(arg: &OsString, input: Option<&OsString>) -> UResult<Option<Settings>> {
+    match parse::parse_obsolete(arg) {
+        Some(Ok(args)) => Ok(Some(Settings::from_obsolete_args(&args, input))),
         None => Ok(None),
-        Some(Err(e)) => Err(USimpleError::new(
-            1,
-            match e {
-                parse::ParseError::OutOfRange => format!(
-                    "invalid number: {}: Numerical result out of range",
-                    args.quote()
-                ),
-                parse::ParseError::Overflow => format!("invalid number: {}", args.quote()),
-                parse::ParseError::Context => format!(
-                    "option used in invalid context -- {}",
-                    args.chars().nth(1).unwrap_or_default()
-                ),
-            },
-        )),
+        Some(Err(e)) => {
+            let arg_str = arg.to_string_lossy();
+            Err(USimpleError::new(
+                1,
+                match e {
+                    parse::ParseError::OutOfRange => format!(
+                        "invalid number: {}: Numerical result out of range",
+                        arg_str.quote()
+                    ),
+                    parse::ParseError::Overflow => format!("invalid number: {}", arg_str.quote()),
+                    // this ensures compatibility to GNU's error message (as tested in misc/tail)
+                    parse::ParseError::Context => format!(
+                        "option used in invalid context -- {}",
+                        arg_str.chars().nth(1).unwrap_or_default()
+                    ),
+                    parse::ParseError::InvalidEncoding => {
+                        format!("bad argument encoding: '{arg_str}'")
+                    }
+                },
+            ))
+        }
     }
 }
 
@@ -410,39 +404,41 @@ fn parse_num(src: &str) -> Result<Signum, ParseSizeError> {
 
 pub fn parse_args(args: impl uucore::Args) -> UResult<Settings> {
     let args_vec: Vec<OsString> = args.collect();
-    let clap_result = match uu_app().try_get_matches_from(args_vec.clone()) {
-        Ok(matches) => {
-            let settings = Settings::from(&matches)?;
-            if !settings.is_default() {
-                // non-default settings can't have obsolete arguments
-                return Ok(settings);
-            }
-            Ok(settings)
-        }
+    let clap_args = uu_app().try_get_matches_from(args_vec.clone());
+    let clap_result = match clap_args {
+        Ok(matches) => Ok(Settings::from(&matches)?),
         Err(err) => Err(err.into()),
     };
 
-    // clap parsing failed or resulted to default -> check for obsolete/deprecated args
-    // argv[0] is always present
-    let second = match args_vec.get(1) {
-        Some(second) => second,
-        None => return clap_result,
-    };
-    let second_str = match second.to_str() {
-        Some(second_str) => second_str,
-        None => {
-            let invalid_string = second.to_string_lossy();
-            return Err(USimpleError::new(
-                1,
-                format!("bad argument encoding: '{invalid_string}'"),
-            ));
-        }
-    };
-    match parse_obsolete(second_str)? {
-        Some(obsolete_args) => Ok(Settings::from_obsolete_args(
-            &obsolete_args,
-            args_vec.get(2),
-        )),
+    // clap isn't able to handle obsolete syntax.
+    // therefore, we want to check further for obsolete arguments.
+    // argv[0] is always present, argv[1] might be obsolete arguments
+    // argv[2] might contain an input file, argv[3] isn't allowed in obsolete mode
+    if args_vec.len() != 2 && args_vec.len() != 3 {
+        return clap_result;
+    }
+
+    // At this point, there are a few possible cases:
+    //
+    //    1. clap has succeeded and the arguments would be invalid for the obsolete syntax.
+    //    2. The case of `tail -c 5` is ambiguous. clap parses this as `tail -c5`,
+    //       but it could also be interpreted as valid obsolete syntax (tail -c on file '5').
+    //       GNU chooses to interpret this as `tail -c5`, like clap.
+    //    3. `tail -f foo` is also ambiguous, but has the same effect in both cases. We can safely
+    //        use the clap result here.
+    //    4. clap succeeded for obsolete arguments starting with '+', but misinterprets them as
+    //       input files (e.g. 'tail +f').
+    //    5. clap failed because of unknown flags, but possibly valid obsolete arguments
+    //        (e.g. tail -l; tail -10c).
+    //
+    // In cases 4 & 5, we want to try parsing the obsolete arguments, which corresponds to
+    // checking whether clap succeeded or the first argument starts with '+'.
+    let possible_obsolete_args = &args_vec[1];
+    if clap_result.is_ok() && !possible_obsolete_args.to_string_lossy().starts_with('+') {
+        return clap_result;
+    }
+    match parse_obsolete(possible_obsolete_args, args_vec.get(2))? {
+        Some(settings) => Ok(settings),
         None => clap_result,
     }
 }
@@ -477,6 +473,7 @@ pub fn uu_app() -> Command {
                 .num_args(0..=1)
                 .require_equals(true)
                 .value_parser(["descriptor", "name"])
+                .overrides_with(options::FOLLOW)
                 .help("Print the file as it grows"),
         )
         .arg(

src/uu/tail/src/parse.rs

@@ -3,6 +3,8 @@
 //  * For the full copyright and license information, please view the LICENSE
 //  * file that was distributed with this source code.
 
+use std::ffi::OsString;
+
 #[derive(PartialEq, Eq, Debug, Copy, Clone)]
 pub struct ObsoleteArgs {
     pub num: u64,
@@ -27,20 +29,31 @@ pub enum ParseError {
     OutOfRange,
     Overflow,
     Context,
+    InvalidEncoding,
 }
 /// Parses obsolete syntax
-/// tail -\[NUM\]\[bl\]\[f\] and tail +\[NUM\]\[bcl\]\[f\] // spell-checker:disable-line
-pub fn parse_obsolete(src: &str) -> Option<Result<ObsoleteArgs, ParseError>> {
-    let mut chars = src.chars();
-    let sign = chars.next()?;
-    if sign != '+' && sign != '-' {
+/// tail -\[NUM\]\[bcl\]\[f\] and tail +\[NUM\]\[bcl\]\[f\] // spell-checker:disable-line
+pub fn parse_obsolete(src: &OsString) -> Option<Result<ObsoleteArgs, ParseError>> {
+    let mut rest = match src.to_str() {
+        Some(src) => src,
+        None => return Some(Err(ParseError::InvalidEncoding)),
+    };
+    let sign = if let Some(r) = rest.strip_prefix('-') {
+        rest = r;
+        '-'
+    } else if let Some(r) = rest.strip_prefix('+') {
+        rest = r;
+        '+'
+    } else {
         return None;
-    }
+    };
 
-    let numbers: String = chars.clone().take_while(|&c| c.is_ascii_digit()).collect();
-    let has_num = !numbers.is_empty();
+    let end_num = rest
+        .find(|c: char| !c.is_ascii_digit())
+        .unwrap_or(rest.len());
+    let has_num = !rest[..end_num].is_empty();
     let num: u64 = if has_num {
-        if let Ok(num) = numbers.parse() {
+        if let Ok(num) = rest[..end_num].parse() {
             num
         } else {
             return Some(Err(ParseError::OutOfRange));
@@ -48,25 +61,30 @@ pub fn parse_obsolete(src: &str) -> Option<Result<ObsoleteArgs, ParseError>> {
     } else {
         10
     };
+    rest = &rest[end_num..];
 
-    let mut follow = false;
-    let mut mode = 'l';
-    let mut first_char = true;
-    for char in chars.skip_while(|&c| c.is_ascii_digit()) {
-        if !has_num && first_char && sign == '-' && (char == 'c' || char == 'f') {
-            // special cases: -c, -f should be handled by clap (are ambiguous)
-            return None;
-        } else if char == 'f' {
-            follow = true;
-        } else if first_char && (char == 'b' || char == 'c' || char == 'l') {
-            mode = char;
-        } else if has_num && sign == '-' {
+    let mode = if let Some(r) = rest.strip_prefix('l') {
+        rest = r;
+        'l'
+    } else if let Some(r) = rest.strip_prefix('c') {
+        rest = r;
+        'c'
+    } else if let Some(r) = rest.strip_prefix('b') {
+        rest = r;
+        'b'
+    } else {
+        'l'
+    };
+
+    let follow = rest.contains('f');
+    if !rest.chars().all(|f| f == 'f') {
+        // GNU allows an arbitrary amount of following fs, but nothing else
+        if sign == '-' && has_num {
             return Some(Err(ParseError::Context));
-        } else {
-            return None;
         }
-        first_char = false;
+        return None;
     }
+
     let multiplier = if mode == 'b' { 512 } else { 1 };
     let num = match num.checked_mul(multiplier) {
         Some(n) => n,
@@ -87,7 +105,7 @@ mod tests {
     #[test]
     fn test_parse_numbers_obsolete() {
         assert_eq!(
-            parse_obsolete("+2c"),
+            parse_obsolete(&OsString::from("+2c")),
             Some(Ok(ObsoleteArgs {
                 num: 2,
                 plus: true,
@@ -96,7 +114,7 @@ mod tests {
             }))
         );
         assert_eq!(
-            parse_obsolete("-5"),
+            parse_obsolete(&OsString::from("-5")),
             Some(Ok(ObsoleteArgs {
                 num: 5,
                 plus: false,
@@ -105,7 +123,7 @@ mod tests {
             }))
         );
         assert_eq!(
-            parse_obsolete("+100f"),
+            parse_obsolete(&OsString::from("+100f")),
             Some(Ok(ObsoleteArgs {
                 num: 100,
                 plus: true,
@@ -114,7 +132,7 @@ mod tests {
             }))
         );
         assert_eq!(
-            parse_obsolete("-2b"),
+            parse_obsolete(&OsString::from("-2b")),
             Some(Ok(ObsoleteArgs {
                 num: 1024,
                 plus: false,
@@ -125,23 +143,47 @@ mod tests {
     }
     #[test]
     fn test_parse_errors_obsolete() {
-        assert_eq!(parse_obsolete("-5n"), Some(Err(ParseError::Context)));
-        assert_eq!(parse_obsolete("-5c5"), Some(Err(ParseError::Context)));
-        assert_eq!(parse_obsolete("-1vzc"), Some(Err(ParseError::Context)));
-        assert_eq!(parse_obsolete("-5m"), Some(Err(ParseError::Context)));
-        assert_eq!(parse_obsolete("-1k"), Some(Err(ParseError::Context)));
-        assert_eq!(parse_obsolete("-1mmk"), Some(Err(ParseError::Context)));
-        assert_eq!(parse_obsolete("-105kzm"), Some(Err(ParseError::Context)));
-        assert_eq!(parse_obsolete("-1vz"), Some(Err(ParseError::Context)));
         assert_eq!(
-            parse_obsolete("-1vzqvq"), // spell-checker:disable-line
+            parse_obsolete(&OsString::from("-5n")),
+            Some(Err(ParseError::Context))
+        );
+        assert_eq!(
+            parse_obsolete(&OsString::from("-5c5")),
+            Some(Err(ParseError::Context))
+        );
+        assert_eq!(
+            parse_obsolete(&OsString::from("-1vzc")),
+            Some(Err(ParseError::Context))
+        );
+        assert_eq!(
+            parse_obsolete(&OsString::from("-5m")),
+            Some(Err(ParseError::Context))
+        );
+        assert_eq!(
+            parse_obsolete(&OsString::from("-1k")),
+            Some(Err(ParseError::Context))
+        );
+        assert_eq!(
+            parse_obsolete(&OsString::from("-1mmk")),
+            Some(Err(ParseError::Context))
+        );
+        assert_eq!(
+            parse_obsolete(&OsString::from("-105kzm")),
+            Some(Err(ParseError::Context))
+        );
+        assert_eq!(
+            parse_obsolete(&OsString::from("-1vz")),
+            Some(Err(ParseError::Context))
+        );
+        assert_eq!(
+            parse_obsolete(&OsString::from("-1vzqvq")), // spell-checker:disable-line
             Some(Err(ParseError::Context))
         );
     }
     #[test]
     fn test_parse_obsolete_no_match() {
-        assert_eq!(parse_obsolete("-k"), None);
-        assert_eq!(parse_obsolete("asd"), None);
-        assert_eq!(parse_obsolete("-cc"), None);
+        assert_eq!(parse_obsolete(&OsString::from("-k")), None);
+        assert_eq!(parse_obsolete(&OsString::from("asd")), None);
+        assert_eq!(parse_obsolete(&OsString::from("-cc")), None);
     }
 }

src/uu/tail/src/paths.rs

@@ -28,8 +28,7 @@ pub struct Input {
 
 impl Input {
     pub fn from<T: AsRef<OsStr>>(string: &T) -> Self {
-        let valid_string = string.as_ref().to_str();
-        let kind = if valid_string.is_some() && valid_string.unwrap() == text::DASH {
+        let kind = if string.as_ref() == Path::new(text::DASH) {
             InputKind::Stdin
         } else {
             InputKind::File(PathBuf::from(string.as_ref()))