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

Fix status code when bad command name is entered #3616

Merged
merged 1 commit into from Dec 3, 2016

Conversation

Projects
None yet
2 participants
@radomirbosak
Contributor

radomirbosak commented Dec 3, 2016

Description

This commit fixes a bug which causes that

fish -c ')'; echo $status

("Illegal command name" error) returns 0. This is inconsistent with
e.g. when trying to run non-existent command:

fish -c 'invalid-command'; echo $status

("Unknown command" error) which correctly returns 127.

A new status code,

STATUS_ILLEGAL_CMD = 123

is introduced - which is returned whenever the 'Illegal command name *'
message is printed.

This commit also adds a test which checks if valid commands return 0,
while commands with illegal name return status code 123.

Fixes #3606.

TODOs:

  • Changes to fish usage are reflected in user documenation/manpages.
  • Tests have been added
  • Decide if new status code STATUS_ILLEGAL_CMD=123 should really be added, or STATUS_UNKNOWN_COMMAND=127 should be reused (as the issue reporter suggested)

What do you think? Should STATUS_UNKNOWN_COMMAND be used instead? I don't have problems rewriting the commit to use it instead of new status code.

And please check the test. I'm not very experienced with C++ .

Fix status code when bad command name is entered
This commit fixes a bug which causes that

   fish -c ')'; echo $status

("Illegal command name" error) returns 0. This is inconsistent with
e.g. when trying to run non-existent command:

   fish -c 'invalid-command'; echo $status

("Unknown command" error) which correctly returns 127.

A new status code,

    STATUS_ILLEGAL_CMD = 123

is introduced - which is returned whenever the 'Illegal command name *'
message is printed.

This commit also adds a test which checks if valid commands return 0,
while commands with illegal name return status code 123.

Fixes #3606.
@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Dec 3, 2016

Member

Thanks for the great contribution. The tests look outstanding!

I like the introduction of STATUS_ILLEGAL_CMD since that seems like a very distinct error from STATUS_UNKNOWN_CMD. The first is for syntactic errors, the second for commands that were not found.

Member

ridiculousfish commented Dec 3, 2016

Thanks for the great contribution. The tests look outstanding!

I like the introduction of STATUS_ILLEGAL_CMD since that seems like a very distinct error from STATUS_UNKNOWN_CMD. The first is for syntactic errors, the second for commands that were not found.

@ridiculousfish ridiculousfish added this to the fish 2.5.0 milestone Dec 3, 2016

@ridiculousfish ridiculousfish merged commit 254762f into fish-shell:master Dec 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Dec 3, 2016

Member

Merged. Thank you!!

Member

ridiculousfish commented Dec 3, 2016

Merged. Thank you!!

@ridiculousfish ridiculousfish added the bug label Dec 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment