return non-zero exit code if there are errors #923
Conversation
| fn new() -> FormatReport { | ||
| FormatReport { file_error_map: HashMap::new() } | ||
| } | ||
|
|
||
| pub fn warning_count(&self) -> usize { | ||
| self.file_error_map.iter().map(|(_, ref errors)| errors.len()).fold(0, |acc, x| acc + x) |
There was a problem hiding this comment.
I am keen to rename warning_count -> error_count.
|
The code looks good, however, I think it is important to distinguish between formatting errors, parsing errors, and errors in operation, and return different codes. I.e., the caller should be able to distinguish between an error like can't open a file and a formatting error like producing an over-long line. And you should document this somewhere (in the README, I guess). |
matklad
force-pushed
the
proper-exit-code
branch
from
d28b7e7 to
bea9c96
Compare
matklad
force-pushed
the
proper-exit-code
branch
from
bea9c96 to
77350e4
Compare
|
@nrc this now returns |
|
looks good, thanks for the changes. I think 1, 2, 3 is OK for now, we can change them later. We should reconfirm that they are right before 1.0. |
|
This interacts pretty poorly with rust.vim, which will bail out by not saving and printing out the entire file contents in the terminal when rustfmt exist with a non-zero return code (see rust-lang/rust.vim#75). |
|
|
|
@matklad: I agree, the correct thing for rustfmt to do is to indicate that it encountered problems. When the problems are terminal (i.e., nothing could be done), then the right thing to do is definitely to exit with an error code. When the problems are really warnings (e.g., one line couldn't be formatted as it was too long, but otherwise formatting completed), I'm not sure that is the right behavior. That said, this is definitely a bug in |
This change updates the acceptable exit codes from `rustfmt` when deciding whether or not to write back the file or display the errors location list. As of rust-lang/rustfmt#923, `rustfmt` exits with either a 0, 1, 2, or 3 depending on what happened. Prior to this upstream change, `rustfmt` would exit 0 when issues that cannot be automatically resolved would occur. Now, the same errors (such as "line exceeded maximum length") result in an exit of 3 (see the [README](line exceeded maximum length) for more details). This change will treat an exit of 3 as "okay" for writing back as `rustfmt` didn't fail (it just couldn't auto-format a nicer solution). Closes rust-lang#68 Closes rust-lang#75 References rust-lang/rustfmt#923
closes #150
@nrc please review