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

#3390: rename hide_parse_errors to show_parse_errors #5961

Merged
merged 1 commit into from
Dec 16, 2023

Conversation

llesha
Copy link
Contributor

@llesha llesha commented Nov 10, 2023

Fix of 3390 issue, changed hide_parse_errors to show_parse_errors

@ytmimi
Copy link
Contributor

ytmimi commented Nov 10, 2023

Thanks for your first contribution to rustfmt 🎉

There was a similar PR to rename fn_args_layout to fn_params_layout and deprecate fn_args_layout (#5387). Let's take a similar approach with this PR and create a set_hide_parse_errors function that encapsulate the logic of issuing a deprecation warning and setting show_parse_errors if it wasn't otherwise set.

Additionally, let's add hide_parse_errors to the HIDE_OPTIONS list (defined in is_hidden_option) to prevent hide_parse_errors from showing up in the output when a user tries to print their config with

rustfmt --print-config default


- **Default value**: `true`
- **Possible values**: `true`, `false`
- **Stable**: Yes
Copy link
Contributor

@ytmimi ytmimi Dec 4, 2023

Choose a reason for hiding this comment

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

hide_parse_errors was an unstable option. The renamed option also needs to be marked as unstable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What tracing issue should be mentioned? The same as the unstable option (#3390)? Unstable option's issue is about renaming it, so logically that issue is resolved in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to create a new tracking issue (I can add the unstable opiton label). We can also leave the tracking issue blank for now and follow up with it in a future PR. Up to you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

created issue #5977

src/config/mod.rs Outdated Show resolved Hide resolved
src/config/mod.rs Outdated Show resolved Hide resolved
Configurations.md Outdated Show resolved Hide resolved
@ytmimi
Copy link
Contributor

ytmimi commented Dec 10, 2023

@llesha we try to avoid merge commits in this repo. Can you please squash all these changes into a single commit and then rebase on the current master.

@llesha llesha force-pushed the master branch 2 times, most recently from c2232ad to f81c6cb Compare December 10, 2023 21:45
Copy link
Contributor

@ytmimi ytmimi left a comment

Choose a reason for hiding this comment

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

Thanks again for your first contribution to rustfmt and for helping to improve the clarity of rustfmt's configuration options!

@ytmimi ytmimi merged commit d739d93 into rust-lang:master Dec 16, 2023
27 checks passed
@llesha llesha changed the title #3390: hide_parse_errors to show_parse_errors #3390: rename hide_parse_errors to show_parse_errors Dec 18, 2023
@ytmimi ytmimi removed the release-notes Needs an associated changelog entry label Jul 6, 2024
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.

None yet

3 participants