Skip to content

nl: match GNU's errors for its numeric and numbering-style option values #14314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
arbelonson-source wants to merge 1 commit into from
Closed

nl: match GNU's errors for its numeric and numbering-style option values #14314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/uu/nl/locales/en-US.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@ nl-help-starting-line-number = first line number on each logical page
nl-help-number-width = use NUMBER columns for line numbers

# Error messages
nl-error-invalid-arguments = Invalid arguments supplied.
nl-error-could-not-read-line = could not read line
nl-error-could-not-write = could not write output
nl-error-line-number-overflow = line number overflow
nl-error-invalid-regex = invalid regular expression
nl-error-invalid-numbering-style = invalid numbering style: '{ $style }'
nl-error-invalid-regex = Invalid regular expression
nl-error-invalid-numbering-style = invalid { $kind } numbering style: '{ $value }'
nl-error-invalid-number-format = invalid line numbering format: '{ $value }'
nl-error-invalid-number = invalid { $kind }: '{ $value }'
nl-error-number-out-of-range = invalid { $kind }: '{ $value }': Numerical result out of range
nl-error-number-too-large = invalid { $kind }: '{ $value }': Value too large for defined data type
nl-error-try-help = Try 'nl --help' for more information.
nl-error-is-directory = { $path }: Is a directory
10 changes: 7 additions & 3 deletions src/uu/nl/locales/fr-FR.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@ nl-help-starting-line-number = premier numéro de ligne sur chaque page logique
nl-help-number-width = utiliser NUMBER colonnes pour les numéros de ligne

# Messages d'erreur
nl-error-invalid-arguments = Arguments fournis invalides.
nl-error-could-not-read-line = impossible de lire la ligne
nl-error-could-not-write = impossible d'écrire la sortie
nl-error-line-number-overflow = débordement du numéro de ligne
nl-error-invalid-regex = expression régulière invalide
nl-error-invalid-numbering-style = style de numérotation invalide : '{ $style }'
nl-error-invalid-regex = Expression régulière invalide
nl-error-invalid-numbering-style = style de numérotation { $kind } invalide : '{ $value }'
nl-error-invalid-number-format = format de numérotation de ligne invalide : '{ $value }'
nl-error-invalid-number = { $kind } invalide : '{ $value }'
nl-error-number-out-of-range = { $kind } invalide : '{ $value }' : Résultat numérique hors limite
nl-error-number-too-large = { $kind } invalide : '{ $value }' : Valeur trop grande pour le type de données défini
nl-error-try-help = Essayez 'nl --help' pour plus d'informations.
nl-error-is-directory = { $path } : Est un répertoire
184 changes: 137 additions & 47 deletions src/uu/nl/src/helper.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,100 @@
// spell-checker:ignore (ToDO) conv

use std::ffi::OsString;
use std::num::IntErrorKind;

use crate::options;
use uucore::translate;

// parse_options loads the options into the settings, returning an array of
// error messages.
use crate::{NumberingStyle, NumberingStyleError, options};

/// GNU reports a plain (no unit suffix) numeric option's own value with its
/// own wording, and immediately, fatally, exits on the first one it finds
/// invalid -- it does not keep parsing the rest of the command line the way
/// it does for the numbering-style options below.
///
/// GNU distinguishes two kinds of "doesn't fit": a value that doesn't fit
/// in `overflow_at`'s own underlying integer width at all (e.g. `-w`'s
/// value is stored in a plain C `int`, so anything outside `i32` overflows
/// it even though it parses as an `i64` just fine) is reported as "Value
/// too large for defined data type"; one that fits that width but falls
/// outside the option's own accepted `range` (e.g. `-w` additionally
/// requires a *positive* value) is reported as "Numerical result out of
/// range" instead. `kind` names which option this is, for the message.
fn parse_nl_number(
value: &str,
kind: &'static str,
overflow_at: std::ops::RangeInclusive<i64>,
range: std::ops::RangeInclusive<i64>,
) -> Result<i64, String> {
match value.parse::<i64>() {
Ok(n) if range.contains(&n) => Ok(n),
Ok(n) if overflow_at.contains(&n) => Err(
translate!("nl-error-number-out-of-range", "kind" => kind, "value" => value.to_owned()),
),
Ok(_) => Err(
translate!("nl-error-number-too-large", "kind" => kind, "value" => value.to_owned()),
),
Err(e)
if matches!(
e.kind(),
IntErrorKind::PosOverflow | IntErrorKind::NegOverflow
) =>
{
Err(
translate!("nl-error-number-too-large", "kind" => kind, "value" => value.to_owned()),
)
}
Err(_) => {
Err(translate!("nl-error-invalid-number", "kind" => kind, "value" => value.to_owned()))
}
}
}

// parse_options loads the options into the settings, returning either the
// first immediately-fatal error found (matching GNU, which stops parsing the
// rest of the command line at that point), or the list of any non-fatal
// numbering-style/format errors collected along the way.
#[allow(clippy::cognitive_complexity)]
pub fn parse_options(settings: &mut crate::Settings, opts: &clap::ArgMatches) -> Vec<String> {
// This vector holds error messages encountered.
pub fn parse_options(
settings: &mut crate::Settings,
opts: &clap::ArgMatches,
) -> Result<Vec<String>, String> {
let mut errs: Vec<String> = vec![];
settings.renumber = opts.get_flag(options::NO_RENUMBER);

if let Some(value) = opts.get_one::<String>(options::LINE_INCREMENT) {
settings.line_increment = parse_nl_number(
value,
"line number increment",
i64::MIN..=i64::MAX,
i64::MIN..=i64::MAX,
)?;
}
if let Some(value) = opts.get_one::<String>(options::JOIN_BLANK_LINES) {
settings.join_blank_lines = parse_nl_number(
value,
"line number of blank lines",
i64::MIN..=i64::MAX,
0..=i64::MAX,
)? as u64;
}
if let Some(value) = opts.get_one::<String>(options::STARTING_LINE_NUMBER) {
settings.starting_line_number = parse_nl_number(
value,
"starting line number",
i64::MIN..=i64::MAX,
i64::MIN..=i64::MAX,
)?;
}
if let Some(value) = opts.get_one::<String>(options::NUMBER_WIDTH) {
settings.number_width = parse_nl_number(
value,
"line number field width",
i64::from(i32::MIN)..=i64::from(i32::MAX),
1..=i64::from(i32::MAX),
)? as usize;
}

if let Some(mut delimiter) = opts
.get_one::<OsString>(options::SECTION_DELIMITER)
.cloned()
Expand All @@ -35,48 +118,55 @@ pub fn parse_options(settings: &mut crate::Settings, opts: &clap::ArgMatches) ->
if let Some(val) = opts.get_one::<OsString>(options::NUMBER_SEPARATOR) {
settings.number_separator.clone_from(val);
}
settings.number_format = opts
.get_one::<String>(options::NUMBER_FORMAT)
.map(Into::into)
.unwrap_or_default();
match opts
.get_one::<String>(options::HEADER_NUMBERING)
.map(String::as_str)
.map(TryInto::try_into)
{
None => {}
Some(Ok(style)) => settings.header_numbering = style,
Some(Err(message)) => errs.push(message),
}
match opts
.get_one::<String>(options::BODY_NUMBERING)
.map(String::as_str)
.map(TryInto::try_into)
{
None => {}
Some(Ok(style)) => settings.body_numbering = style,
Some(Err(message)) => errs.push(message),
}
match opts
.get_one::<String>(options::FOOTER_NUMBERING)
.map(String::as_str)
.map(TryInto::try_into)
{
None => {}
Some(Ok(style)) => settings.footer_numbering = style,
Some(Err(message)) => errs.push(message),
}
if let Some(&num) = opts.get_one::<u64>(options::NUMBER_WIDTH) {
settings.number_width = num as usize;
}
if let Some(num) = opts.get_one::<u64>(options::JOIN_BLANK_LINES) {
settings.join_blank_lines = *num;
}
if let Some(num) = opts.get_one::<i64>(options::LINE_INCREMENT) {
settings.line_increment = *num;

// GNU reports these four in the order they were actually given on the
// command line, not in any fixed order, so each is paired with its own
// position for a final sort.
let mut style_errs: Vec<(usize, String)> = vec![];

if let Some(format) = opts.get_one::<String>(options::NUMBER_FORMAT) {
match format.as_str() {
"ln" | "rn" | "rz" => settings.number_format = format.clone().into(),
_ => style_errs.push((
opts.index_of(options::NUMBER_FORMAT).unwrap_or(0),
translate!("nl-error-invalid-number-format", "value" => format.clone()),
)),
}
}
if let Some(num) = opts.get_one::<i64>(options::STARTING_LINE_NUMBER) {
settings.starting_line_number = *num;

for (opt, kind, field) in [
(
options::HEADER_NUMBERING,
"header",
&mut settings.header_numbering,
),
(
options::BODY_NUMBERING,
"body",
&mut settings.body_numbering,
),
(
options::FOOTER_NUMBERING,
"footer",
&mut settings.footer_numbering,
),
] {
if let Some(style) = opts.get_one::<String>(opt) {
match NumberingStyle::try_from(style.as_str()) {
Ok(numbering) => *field = numbering,
Err(NumberingStyleError::InvalidRegex) => {
return Err(translate!("nl-error-invalid-regex"));
}
Err(NumberingStyleError::InvalidStyle) => style_errs.push((
opts.index_of(opt).unwrap_or(0),
translate!("nl-error-invalid-numbering-style", "kind" => kind, "value" => style.clone()),
)),
}
}
}
errs

style_errs.sort_by_key(|(index, _)| *index);
errs.extend(style_errs.into_iter().map(|(_, message)| message));

Ok(errs)
}
59 changes: 39 additions & 20 deletions src/uu/nl/src/nl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,19 @@ enum NumberingStyle {
Regex(Box<regex::bytes::Regex>),
}

/// Which of the two ways a numbering style argument can fail: with an
/// invalid style word, or -- for a `p`-prefixed one -- an invalid regular
/// expression. The caller picks the message: GNU's wording for the first
/// depends on which of the header/body/footer/format options it came
/// from, and its "Try --help" hint depends on whether it's a style word
/// at all, so this can't be a single, already-translated `String`.
enum NumberingStyleError {
InvalidStyle,
InvalidRegex,
}

impl TryFrom<&str> for NumberingStyle {
type Error = String;
type Error = NumberingStyleError;

fn try_from(s: &str) -> Result<Self, Self::Error> {
match s {
Expand All @@ -90,9 +101,9 @@ impl TryFrom<&str> for NumberingStyle {
"n" => Ok(Self::None),
_ if s.starts_with('p') => match regex::bytes::Regex::new(&s[1..]) {
Ok(re) => Ok(Self::Regex(Box::new(re))),
Err(_) => Err(translate!("nl-error-invalid-regex")),
Err(_) => Err(NumberingStyleError::InvalidRegex),
},
_ => Err(translate!("nl-error-invalid-numbering-style", "style" => s)),
_ => Err(NumberingStyleError::InvalidStyle),
}
}
}
Expand Down Expand Up @@ -220,17 +231,26 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let mut settings = Settings::default();

// Update the settings from the command line options, and terminate the
// program if some options could not successfully be parsed.
let parse_errors = helper::parse_options(&mut settings, &matches);
if !parse_errors.is_empty() {
return Err(USimpleError::new(
1,
format!(
"{}\n{}",
translate!("nl-error-invalid-arguments"),
parse_errors.join("\n")
),
));
// program if some options could not successfully be parsed. GNU stops
// immediately, with no "Try --help" hint, on the first option whose
// own value is invalid (an immediately-fatal `Err` here); it instead
// collects every bad numbering-style/format option and reports them
// all together with a single trailing hint at the end.
match helper::parse_options(&mut settings, &matches) {
Ok(errs) if errs.is_empty() => {}
Ok(errs) => {
// The eventual `show_error!` only prefixes the message's first
// line with "nl: "; every other error line needs its own.
let mut lines = errs.into_iter();
let mut message = lines.next().unwrap_or_default();
for err in lines {
message.push_str(&format!("\nnl: {err}"));
}
message.push('\n');
message.push_str(&translate!("nl-error-try-help"));
return Err(USimpleError::new(1, message));
}
Err(err) => return Err(USimpleError::new(1, err)),
}

let files: Vec<OsString> = match matches.get_many::<OsString>(options::FILE) {
Expand Down Expand Up @@ -329,23 +349,22 @@ pub fn uu_app() -> Command {
.long(options::LINE_INCREMENT)
.help(translate!("nl-help-line-increment"))
.value_name("NUMBER")
.value_parser(clap::value_parser!(i64)),
.allow_hyphen_values(true),
)
.arg(
Arg::new(options::JOIN_BLANK_LINES)
.short('l')
.long(options::JOIN_BLANK_LINES)
.help(translate!("nl-help-join-blank-lines"))
.value_name("NUMBER")
.value_parser(clap::value_parser!(u64)),
.allow_hyphen_values(true),
)
.arg(
Arg::new(options::NUMBER_FORMAT)
.short('n')
.long(options::NUMBER_FORMAT)
.help(translate!("nl-help-number-format"))
.value_name("FORMAT")
.value_parser(["ln", "rn", "rz"]),
.value_name("FORMAT"),
)
.arg(
Arg::new(options::NO_RENUMBER)
Expand All @@ -368,15 +387,15 @@ pub fn uu_app() -> Command {
.long(options::STARTING_LINE_NUMBER)
.help(translate!("nl-help-starting-line-number"))
.value_name("NUMBER")
.value_parser(clap::value_parser!(i64)),
.allow_hyphen_values(true),
)
.arg(
Arg::new(options::NUMBER_WIDTH)
.short('w')
.long(options::NUMBER_WIDTH)
.help(translate!("nl-help-number-width"))
.value_name("NUMBER")
.value_parser(clap::value_parser!(u64).range(1..=(i32::MAX as u64))),
.allow_hyphen_values(true),
)
}

Expand Down
Loading
Loading