Skip to content

Check shell return code in addition to output#19

Merged
lucerion merged 2 commits intoruby-formatter:masterfrom
13k:check-shell-return-codes
May 22, 2018
Merged

Check shell return code in addition to output#19
lucerion merged 2 commits intoruby-formatter:masterfrom
13k:check-shell-return-codes

Conversation

@13k
Copy link
Copy Markdown
Contributor

@13k 13k commented May 21, 2018

When the rufo command does not exist, or outputs something with a non-success exit code, the current failure check considers the run as successful, replacing the original buffer with the error message.

By checking the command return code (v:shell_error), it correctly displays the message in the error buffer and keeps the original buffer intact.

@13k 13k force-pushed the check-shell-return-codes branch 3 times, most recently from 9382dde to 9b37795 Compare May 21, 2018 16:36
@splattael splattael requested a review from lucerion May 21, 2018 17:58
@splattael
Copy link
Copy Markdown
Member

@13k Nice, thank you! 👍

I can confirm the behaviour described:

Before

rufo-vim-buffer

After

rufo-vim-buffer-fixed

@lucerion I suspect that the commit 60241f8 broke it. WDYT?

Copy link
Copy Markdown
Member

@splattael splattael left a comment

Choose a reason for hiding this comment

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

Forgot to approve 💜

@13k 13k force-pushed the check-shell-return-codes branch from 9b37795 to c770ef4 Compare May 21, 2018 19:08
@13k
Copy link
Copy Markdown
Contributor Author

13k commented May 21, 2018

@splattael I've updated the PR, it was incorrectly assuming non-zero is an error, but rufo would exit with code 3 for "formatting would change the input". So I'm checking for status !=0 && status != 3.

Something still bugs me, though, that is the "first output line contains the word Error" check. If I'm formatting code that has Error in the first line, it will detect the output as an error, despite the status codes. I think it should only rely on the status codes, not the output.

@13k
Copy link
Copy Markdown
Contributor Author

13k commented May 21, 2018

Example:

peek 2018-05-21 16-21

@lucerion
Copy link
Copy Markdown
Member

lucerion commented May 22, 2018

@13k it's a hack that can be completely deleted and we can use exit codes only. When I've added it (#5) there was no exit codes in rufo (ruby-formatter/rufo@068bf0b).

@13k 13k force-pushed the check-shell-return-codes branch from 900327f to 6a21199 Compare May 22, 2018 14:27
@13k
Copy link
Copy Markdown
Contributor Author

13k commented May 22, 2018

@lucerion @splattael Updated the PR. It now checks only the exit code.

Thanks for your feedback!

@splattael
Copy link
Copy Markdown
Member

@13k LGTM 👍 but @lucerion should approve as well 💚

@lucerion lucerion merged commit c0a59cd into ruby-formatter:master May 22, 2018
@lucerion
Copy link
Copy Markdown
Member

@13k good job! Thank you!

@13k 13k deleted the check-shell-return-codes branch May 22, 2018 23:52
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.

3 participants