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

#3402 set the exit code for single commands #3632

Merged
merged 7 commits into from
Aug 1, 2020
Merged

#3402 set the exit code for single commands #3632

merged 7 commits into from
Aug 1, 2020

Conversation

dyuri
Copy link
Contributor

@dyuri dyuri commented Jun 22, 2020

Attempt to fix #3402 .

$ touch file_exists
$ xonsh -c "rm file_exists"
$ _.rtn
0
$ xonsh -c "rm no_such_file"
/usr/bin/rm: cannot remove 'no_such_file': No such file or directory
$ _.rtn
1
$ xonsh -c "sys.exit(123)"
$ _.rtn
123

(But it still does not seem to fix the issue with Fugitive.)

@scopatz
Copy link
Member

scopatz commented Jul 28, 2020

Thanks for putting this in @dyuri! Can you add a case or two here to the integration tests?

@dyuri
Copy link
Contributor Author

dyuri commented Jul 28, 2020

Sure, on it.

@dyuri
Copy link
Contributor Author

dyuri commented Jul 28, 2020

(I have no access to any windows machine so I've added @skip_if_on_windows for now.)

@scopatz
Copy link
Member

scopatz commented Jul 28, 2020

Thanks! I think skipping windows makes sense for this feature. Also, can you please add a news item?

@dyuri
Copy link
Contributor Author

dyuri commented Jul 28, 2020

News entry added.

@dyuri
Copy link
Contributor Author

dyuri commented Jul 29, 2020

Sorry, I just noticed that on macOS ls returns 1 for non-existing entry (but I created the tests under linux where it returns 2). I'll replace it with something else.

@dyuri
Copy link
Contributor Author

dyuri commented Jul 29, 2020

Ok, now it seems to be fine, for some reason the checks for this PR used the old code (?), but in my fork they are all OK now: https://github.com/dyuri/xonsh/actions/runs/186758594

@scopatz
Copy link
Member

scopatz commented Aug 1, 2020

So I tried restarting and it didn't do anything. Maybe try merging from master?

@dyuri
Copy link
Contributor Author

dyuri commented Aug 1, 2020

Yep, saw that, trying.

@scopatz scopatz merged commit 8b933da into xonsh:master Aug 1, 2020
@scopatz
Copy link
Member

scopatz commented Aug 1, 2020

Thanks!

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

Successfully merging this pull request may close these issues.

xonsh always returns zero when used as the shell in vim/neovim
2 participants