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

Fix for #3805 - wrap macro that starts with nested body blocks #5582

Conversation

davidBar-On
Copy link
Contributor

@davidBar-On davidBar-On commented Nov 2, 2022

Fixes #3805

For wrapping macros with body block that starts with more than one {, i.e. starts with "nested blocks". The issue cause was that the nested block(s) { were not considered for calculating the remaining line width.

src/macros.rs Outdated Show resolved Hide resolved
@ytmimi
Copy link
Contributor

ytmimi commented Nov 9, 2022

Thanks for your help on this! I left two minor comments inline. Let me know your thoughts on both points.

@davidBar-On
Copy link
Contributor Author

Accepted the suggested changes, with minor change to the first comment. Note also my note about the formatted output of new nest case with multi {}.

src/macros.rs Outdated Show resolved Hide resolved
@davidBar-On davidBar-On force-pushed the issue-3805-macro-wrap-of-one-char-beyond-max_width branch from 804a4b6 to 643f32c Compare November 15, 2022 19:41
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 for the help fixing this issue, and for applying the feedback. I think we're good to go here!

@calebcartwright
Copy link
Member

I'm good with this change, but it will need to be version gated

@davidBar-On is that something you'd be willing to do when you have time?

@davidBar-On davidBar-On force-pushed the issue-3805-macro-wrap-of-one-char-beyond-max_width branch from 643f32c to b1fecdd Compare February 6, 2023 20:03
@davidBar-On
Copy link
Contributor Author

I'm good with this change, but it will need to be version gated

Added the version gating.
Also rebased and consolidated all changes into one commit.

Based on version other gating code, I used if context.config.version() == Version::Two, although I think that it may be better to use if context.config.version() != Version::One which is also good for Version::Three.

@ytmimi ytmimi force-pushed the issue-3805-macro-wrap-of-one-char-beyond-max_width branch from b1fecdd to b9cc509 Compare January 28, 2024 23:47
@ytmimi
Copy link
Contributor

ytmimi commented Jan 29, 2024

This one has been marked as ready-to-merge for a while, and the fix has been version gated. Everything continues to pass after rebasing so I think we're good to go here.

@ytmimi ytmimi merged commit 7bedb9f into rust-lang:master Jan 29, 2024
27 checks passed
@ytmimi ytmimi added release-notes Needs an associated changelog entry and removed pr-ready-to-merge labels Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes Needs an associated changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

macro fails to be wrapped if it is exactly one character beyond max_width
3 participants