move fish_command_not_found exit code and redirection handling up to the error handler function #9517
Conversation
|
Looks like I have some tests to fix. But input on whether this is desired/acceptable/usable style is welcome. |
| $fish -C 'function fish_command_not_found; echo cmd-not-found; end' -c "nonexistent-command-1234 1 2 3 4" | ||
| ##CHECKERR: cmd-not-found | ||
| #CHECKERR: fish: Unknown command: nonexistent-command-1234 | ||
| #CHECKERR: nonexistent-command-1234 1 2 3 4 | ||
| #CHECKERR: ^~~~~~~~~~~~~~~~~~~~~~~^ | ||
| $fish -C 'function fish_command_not_found; echo command-not-found $argv; end' -c "nonexistent-command-abcd foo bar baz" | ||
| #CHECKERR: command-not-found nonexistent-command-abcd foo bar baz | ||
| #CHECKERR: fish: | ||
| ##CHECKERR: command-not-found nonexistent-command-abcd foo bar baz | ||
| #CHECKERR: fish: Unknown command: nonexistent-command-abcd |
There was a problem hiding this comment.
this is a regression, unfortunately. need to figure out why this particular needed case fails.
There was a problem hiding this comment.
This fails because this runs a fish - $fish above - that isn't interactive.
So because you only made it execute fish_command_not_found if fish is interactive, and this tries to override fish_command_not_found, it no longer works.
I'm not sure if that's okay.
There was a problem hiding this comment.
Yes, I'm not 100% that this doesn't need more tweaks, but I felt it was a start. A bunch of the error responses changed because the error return was moved to the fish function itself, rather than propagating out of the C++ (as far I understood it). Help is absolutely welcome.
| # CHECKERR: {{.*}}: Unknown command: '{ echo; echo }' | ||
| # CHECKERR: {{.*}}: '{ ... }' is not supported for grouping commands. Please use 'begin; ...; end' | ||
| # CHECKERR: {{.*}}: Unknown command: '{ echo; echo }''{ ... }' is not supported for grouping commands. Please use 'begin; ...; end' |
There was a problem hiding this comment.
this is also a regression. these lines shouldn't be joined.
| $fish -C 'function fish_command_not_found; echo cmd-not-found; end' -ic "nonexistent-command-1234 1 2 3 4" | ||
| #CHECKERR: cmd-not-found | ||
| #CHECKERR: fish: | ||
| $fish -C 'function fish_command_not_found; echo cmd-not-found; end' -c "nonexistent-command-1234 1 2 3 4" |
| } else { | ||
| // Redirect to stderr _if there's no handler_ | ||
| io.append_from_specs({redirection_spec_t{STDOUT_FILENO, redirection_mode_t::fd, L"2"}}, | ||
| L""); | ||
| // If we have no handler, just print it as a normal error. | ||
| error = _(L"Unknown command:"); | ||
| if (!event_args.empty()) { |
There was a problem hiding this comment.
This needs a newline (after the if which github won't allow me to mark):
error.push_back(L'\n');I believe that's a pre-existing bug that this just happens to run into.
| @@ -13,6 +13,7 @@ end | |||
|
|
|||
| function __fish_default_command_not_found_handler | |||
| printf (_ "fish: Unknown command: %s\n") (string escape -- $argv[1]) >&2 | |||
| return 127 | |||
There was a problem hiding this comment.
All the other handlers should also default to return 127.
|
Alright, closing this since there are open issues and it's conflicting. If you get back to it we can of course reopen (or you can file a new PR). |
Description
This moves the choice of redirection to stderr and the 127 exit code up to the error handler function if defined. Fallback code unchanged. Default error handler config is also updated to maintain backwards compatibility. This matches, as best I can tell, the behavior of zsh/bash. fixes pkgxdev/pkgx#344
Fixes issue #7902
TODOs: