Skip to content
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.

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

builtin commands ignore redirects when printing man pages #10274

Closed
Humandoodlebug opened this issue Jan 30, 2024 · 0 comments · Fixed by #10276
Closed

builtin commands ignore redirects when printing man pages #10274

Humandoodlebug opened this issue Jan 30, 2024 · 0 comments · Fixed by #10276
Labels
Milestone

Comments

@Humandoodlebug
Copy link
Contributor

fish, version 3.7.0-1382-g54bc19691
OS: Arch Linux, kernel: 6.6.14-1-lts
Terminal: Konsole 23.08.4-2
Same behaviour with/without third-party customisations.

Noticed when debugging a local test failure in builtinbuiltin.fish. The test script does this:

builtin -- -q &| grep -q "builtin - run a builtin command\|fish: builtin: missing man page"
echo $status
#CHECK: 0

This works when the man page is missing, but fails when it is present, even though the match string accounts for this case.
The failure is caused by the function that prints the man page:

pub fn builtin_print_help_error(
parser: &Parser,
streams: &mut IoStreams,
cmd: &wstr,
error_message: &wstr,
) {
// This won't ever work if no_exec is set.
if no_exec() {
return;
}
let name_esc = escape(cmd);
let mut cmd = sprintf!("__fish_print_help %ls ", &name_esc);
let mut ios = IoChain::new();
if !error_message.is_empty() {
cmd.push_utfstr(&escape(error_message));
// If it's an error, redirect the output of __fish_print_help to stderr
ios.push(Arc::new(IoFd::new(STDOUT_FILENO, STDERR_FILENO)));
}
let res = parser.eval(&cmd, &ios);
if res.status.normal_exited() && res.status.exit_code() == 2 {
streams
.err
.append(wgettext_fmt!(BUILTIN_ERR_MISSING_HELP, name_esc, name_esc));
}
}

Notice towards the end of the function that if the man page is missing, we simply append the error message to stderr:

    if res.status.normal_exited() && res.status.exit_code() == 2 {
        streams
            .err
            .append(wgettext_fmt!(BUILTIN_ERR_MISSING_HELP, name_esc, name_esc));
    }

All good so far. But taking a look at the code running the man page, we do something entirely different:

    let mut ios = IoChain::new();
    if !error_message.is_empty() {
        cmd.push_utfstr(&escape(error_message));
        // If it's an error, redirect the output of __fish_print_help to stderr
        ios.push(Arc::new(IoFd::new(STDOUT_FILENO, STDERR_FILENO)));
    }
    let res = parser.eval(&cmd, &ios);

Since we're creating a new IoChain, any existing redirects held in streams.io_chain are ignored, leading to the help text being printed to stdout (or stderr if there is help text) regardless of whether it is being redirected. This causes the test to fail (and as a side note, this behaviour is pretty inconsistent).

I believe this also affects other builtins, for example read --help &> /dev/null will still print the man page to stdout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants