Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/uu/hostid/src/hostid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ extern "C" {

#[uucore::main]
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
uu_app().get_matches_from(args);
uu_app().try_get_matches_from(args)?;
hostid();
Ok(())
}
Expand Down
8 changes: 1 addition & 7 deletions src/uu/pr/src/pr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,13 +386,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let opt_args = recreate_arguments(&args);

let mut command = uu_app();
let matches = match command.try_get_matches_from_mut(opt_args) {
Ok(m) => m,
Err(e) => {
e.print()?;
return Ok(());
}
};
let matches = command.try_get_matches_from_mut(opt_args)?;

let mut files = matches
.get_many::<String>(options::FILES)
Expand Down
10 changes: 10 additions & 0 deletions src/uu/tee/src/tee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@ pub fn uu_app() -> Command {
.override_usage(format_usage(USAGE))
.after_help(AFTER_HELP)
.infer_long_args(true)
// Since we use value-specific help texts for "--output-error", clap's "short help" and "long help" differ.
Copy link
Member

@tertsdiepraam tertsdiepraam Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this change. It's more compatible, sure, but is it better? I want to limit all sorts of edge cases in the code and might prefer disabling that test here. Or can we force the bit for --output-error to always be shown?

Note that this will be fixed by uutils-args too by the way, so maybe it just doesn't matter much either way.

(Don't consider this a blocking review)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand? All that this piece of code does is force the bit for --output-error to always be shown, just like you suggest.

// However, this is something that the GNU tests explicitly test for, so we *always* show the long help instead.
.disable_help_flag(true)
.arg(
Arg::new("--help")
.short('h')
.long("help")
.help("Print help")
.action(ArgAction::HelpLong)
)
.arg(
Arg::new(options::APPEND)
.long(options::APPEND)
Expand Down
9 changes: 9 additions & 0 deletions tests/by-util/test_hostid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,12 @@ fn test_normal() {
let re = Regex::new(r"^[0-9a-f]{8}").unwrap();
new_ucmd!().succeeds().stdout_matches(&re);
}

#[test]
fn test_invalid_flag() {
new_ucmd!()
.arg("--invalid-argument")
.fails()
.no_stdout()
.code_is(1);
}
9 changes: 9 additions & 0 deletions tests/by-util/test_pr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ fn valid_last_modified_template_vars(from: DateTime<Utc>) -> Vec<Vec<(String, St
.collect()
}

#[test]
fn test_invalid_flag() {
new_ucmd!()
.arg("--invalid-argument")
.fails()
.no_stdout()
.code_is(1);
}

#[test]
fn test_without_any_options() {
let test_file_path = "test_one_page.log";
Expand Down
16 changes: 16 additions & 0 deletions tests/by-util/test_tee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,22 @@ fn test_invalid_arg() {
new_ucmd!().arg("--definitely-invalid").fails().code_is(1);
}

#[test]
fn test_short_help_is_long_help() {
// I can't believe that this test is necessary.
let help_short = new_ucmd!()
.arg("-h")
.succeeds()
.no_stderr()
.stdout_str()
.to_owned();
new_ucmd!()
.arg("--help")
.succeeds()
.no_stderr()
.stdout_is(help_short);
}

#[test]
fn test_tee_processing_multiple_operands() {
// POSIX says: "Processing of at least 13 file operands shall be supported."
Expand Down
11 changes: 4 additions & 7 deletions util/build-gnu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -279,14 +279,11 @@ sed -i "s|\$PACKAGE_VERSION|[0-9]*|g" tests/rm/fail-2eperm.sh tests/mv/sticky-to
# with the option -/ is used, clap is returning a better error than GNU's. Adjust the GNU test
sed -i -e "s~ grep \" '\*/'\*\" err || framework_failure_~ grep \" '*-/'*\" err || framework_failure_~" tests/misc/usage_vs_getopt.sh
sed -i -e "s~ sed -n \"1s/'\\\/'/'OPT'/p\" < err >> pat || framework_failure_~ sed -n \"1s/'-\\\/'/'OPT'/p\" < err >> pat || framework_failure_~" tests/misc/usage_vs_getopt.sh
# Ignore some binaries (not built)
# And change the default error code to 2
# see issue #3331 (clap limitation).
# Upstream returns 1 for most of the program. We do for cp, truncate & pr
# So, keep it as it
sed -i -e "s/rcexp=1$/rcexp=2\n case \"\$prg\" in chcon|runcon) return;; esac/" -e "s/rcexp=125 ;;/rcexp=2 ;;\ncp|truncate|pr) rcexp=1;;/" tests/misc/usage_vs_getopt.sh
# Ignore runcon, it needs some extra attention
# For all other tools, we want drop-in compatibility, and that includes the exit code.
sed -i -e "s/rcexp=1$/rcexp=1\n case \"\$prg\" in runcon|stdbuf) return;; esac/" tests/misc/usage_vs_getopt.sh
# GNU has option=[SUFFIX], clap is <SUFFIX>
sed -i -e "s/cat opts/sed -i -e \"s| <.\*>$||g\" opts/" tests/misc/usage_vs_getopt.sh
sed -i -e "s/cat opts/sed -i -e \"s| <.\*$||g\" opts/" tests/misc/usage_vs_getopt.sh
# for some reasons, some stuff are duplicated, strip that
sed -i -e "s/provoked error./provoked error\ncat pat |sort -u > pat/" tests/misc/usage_vs_getopt.sh

Expand Down