close
Skip to content

#3390: rename hide_parse_errors to show_parse_errors#5961

Merged
ytmimi merged 1 commit into
rust-lang:masterfrom
waterstopper:master
Dec 16, 2023
Merged

#3390: rename hide_parse_errors to show_parse_errors#5961
ytmimi merged 1 commit into
rust-lang:masterfrom
waterstopper:master

Conversation

@waterstopper

@waterstopper waterstopper commented Nov 10, 2023

Copy link
Copy Markdown
Contributor

Fix of 3390 issue, changed hide_parse_errors to show_parse_errors

@ytmimi

ytmimi commented Nov 10, 2023

Copy link
Copy Markdown
Contributor

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

ytmimi
ytmimi reviewed Dec 4, 2023
View reviewed changes
Comment thread Configurations.md Outdated

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

@ytmimi ytmimi Dec 4, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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.

@waterstopper waterstopper Dec 6, 2023

Copy link
Copy Markdown
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.

@ytmimi ytmimi Dec 6, 2023

Copy link
Copy Markdown
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!

@waterstopper waterstopper Dec 8, 2023

Copy link
Copy Markdown
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

ytmimi
ytmimi reviewed Dec 4, 2023
View reviewed changes
Comment thread src/config/mod.rs Outdated
ytmimi
ytmimi reviewed Dec 6, 2023
View reviewed changes
Comment thread src/config/mod.rs Outdated
@waterstopper waterstopper mentioned this pull request Dec 8, 2023
Open
4 tasks
ytmimi
ytmimi reviewed Dec 9, 2023
View reviewed changes
Comment thread Configurations.md Outdated
@ytmimi

ytmimi commented Dec 10, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@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.

@waterstopper
waterstopper force-pushed the master branch 2 times, most recently from c2232ad to f81c6cb Compare December 10, 2023 21:45
@ytmimi ytmimi added pr-ready-to-merge Status: PR is largely ready for merge, waiting for secondary review / last nits release-notes Needs an associated changelog entry and removed pr-waiting-on-author labels Dec 14, 2023
ytmimi
ytmimi approved these changes Dec 16, 2023
View reviewed changes

@ytmimi ytmimi left a comment

Copy link
Copy Markdown
Contributor

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
@ytmimi ytmimi removed the pr-ready-to-merge Status: PR is largely ready for merge, waiting for secondary review / last nits label Dec 16, 2023
@ytmimi ytmimi mentioned this pull request Dec 16, 2023
Closed
@waterstopper waterstopper 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
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Jul 11, 2024
@tcharding
rustfmt: Use show_parse_errors
e2c3694 
There are no behaviour changes with this patch applied.

In rust-lang/rustfmt#5961 the
`hide_parse_errors` was changed to `show_parse_errors` with default
`true`.

Since we run the formatter with nightly we are getting the following
warning

 Warning: the `hide_parse_errors` option is deprecated. Use
 `show_parse_errors` instead

Rename the variable and keep the default value `true`.
@tcharding tcharding mentioned this pull request Jul 11, 2024
Merged
apoelstra added a commit to rust-bitcoin/rust-bitcoin that referenced this pull request Jul 12, 2024
@apoelstra
Merge #3006: rustfmt: Use show_parse_errors
beab1dd 
e2c3694 rustfmt: Use show_parse_errors (Tobin C. Harding)

Pull request description:

  There are no behaviour changes with this patch applied.

  In rust-lang/rustfmt#5961 the `hide_parse_errors` was changed to `show_parse_errors` with default `true`.

  Since we run the formatter with nightly we are getting the following warning

   Warning: the `hide_parse_errors` option is deprecated. Use
   `show_parse_errors` instead

  Rename the variable and keep the default value `true`.

ACKs for top commit:
  Kixunil:
    ACK e2c3694
  storopoli:
    ACK e2c3694
  apoelstra:
    ACK e2c3694

Tree-SHA512: f75f3abf194d65e0ef2a1bef2544bc94ed1c4911b94cbc6bc5700ecb8e6ec87c5564b9711a7dde8ca1a83400e1bf3154d25574a485e9893049be0a46f2c95e33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ytmimi ytmimi ytmimi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@waterstopper @ytmimi @rustbot