Skip to content

dart format should exit non-zero if the source code has errors #1035

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

Closed
natebosch opened this issue Jun 15, 2021 · 5 comments · Fixed by #1036
Closed

dart format should exit non-zero if the source code has errors #1035

natebosch opened this issue Jun 15, 2021 · 5 comments · Fixed by #1036

Comments

@natebosch
Copy link
Member

echo 'not dart' | dartfmt and echo 'not dart' | dart format both produce equivalent error messages, but dartfmt exits with code 65 while dart format exits with 0.

We use the exit code in dart-vim-plugin to know whether the output should be treated as the formatted file, or as errors. Unfortunately in vim it isn't trivial to distinguish between stderr and stdout so using the output stream to determine success would take some additional hacking and running the command twice.

@mit-mit
Copy link
Member

mit-mit commented Jun 16, 2021

cc @munificent

@munificent
Copy link
Member

@bkonyi, the FormatCommand in dart_style should be setting the exit code (test here. Is it possible the dart command isn't plumbing it through?

@bkonyi
Copy link
Contributor

bkonyi commented Jun 17, 2021

@munificent we set the exit code based on the return value from runCommand. If you're setting the dart:io exitCode it'll be ignored.

@munificent munificent transferred this issue from dart-lang/sdk Jun 17, 2021
munificent added a commit that referenced this issue Jun 18, 2021
It reads from stdin asynchronously. Before this fix, the FormatCommand
did not wait for that to complete before returning the exit code, so it
would return before stdin had been formatted and any parse error
detected.

Fix #1035.
@munificent munificent reopened this Jun 21, 2021
@munificent
Copy link
Member

This is fixed in the dart_style repo, but I'm going to keep this issue open until I've rolled the fix into the SDK.

@munificent
Copy link
Member

This is in the SDK and published now.

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 a pull request may close this issue.

4 participants