rustfmt icon indicating copy to clipboard operation
rustfmt copied to clipboard

Correct newline style in json emitter

Open davidBar-On opened this issue 3 years ago • 3 comments

Suggested fix for issue #4273. The main change is in add_misformatted_file() to use the proper newline styles in each of the original/expected fields in each MismatchedBlock. The original code's newline style is used for original while the config newline_style is used for expected.

The other changes are to add parameters to functions/structures to allow passing the config newline_style value to add_misformatted_file(). The new parameters required adding them to function calls in some of the emitter/json.rs test cases. In all these cases default values used.

Test new cases were added to emitter/json.rs, assuming there is no way to verify the json output using test cases files.

The approach taken is that add_misformatted_file() gets the proper newline strings and continues to add the newline to the end of the original/expected strings, using these values instead of constant \n. A simpler alternative may have been to continue adding \n to the end of the strings and then call apply_newline_style() to convert them to the proper newline style. However, that approach seem to be problematic from performance point of view, as the call is done separately for each original/expected field for each MismatchedBlock.

(This is probably the last new PR I am submitting in a while. I know it is a lot of effort and time to review and verify the changes, but I had the time and energy lately to try resolving some of the open issues. Still, if there are issues I can help resolving I will be happy to do that.)

davidBar-On avatar Mar 08 '21 08:03 davidBar-On

(This is probably the last new PR I am submitting in a while. I know it is a lot of effort and time to review and verify the changes, but I had the time and energy lately to try resolving some of the open issues. Still, if there are issues I can help resolving I will be happy to do that.)

Thanks for letting me know, and thanks for all your contributions! Will you still be available to address any review items on your open PRs and help get them over the finish line?

calebcartwright avatar Mar 09 '21 02:03 calebcartwright

Will you still be available to address any review items on your open PRs and help get them over the finish line?

Yes, I have the time. Actually, as I wrote, if there are issues you think I may be able to help with I will be happy to do that.

There are two reasons to what I wrote: one is that I didn't find other help wanted issues that I thought I can help with. The other reason is that I fill that lately I may have overloaded you with new PRs so it seems I should take a break (I see that you do large part of the reviews on weekends).

davidBar-On avatar Mar 09 '21 07:03 davidBar-On

Test new cases were added to emitter/json.rs, assuming there is no way to verify the json output using test cases files.

Still haven't gotten to this one yet, but note it is possible to use input source files and json output files. Some examples for emit modes can be found in https://github.com/rust-lang/rustfmt/tree/master/tests/writemode

calebcartwright avatar Sep 09 '21 00:09 calebcartwright