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 #4471 and #5138 #5976

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix #4471 and #5138 #5976

wants to merge 2 commits into from

Conversation

ahl
Copy link
Contributor

@ahl ahl commented Dec 7, 2023

Fixes #4471 and #5138

I now see that there have been several efforts to fix these (#4524 and #5483), but this takes a pretty different approach. I'm submitting this because I think there may be advantages to this approach. In particular, it may be preferable to not split a \u{123} escape sequence at either brace.

I got here because I hit a variant of this when formatting strings of serialized JSON and rustfmt was splitting the line in the middle of a \" escape.

@ytmimi
Copy link
Contributor

ytmimi commented Jun 26, 2024

@ahl I still haven't had a chance to review this, but I saw the GitHub notification about the PR being updated. I wanted to mention that we strongly discourage merge commits and prefer if you'd rebase your branch to bring it up to date

@ahl
Copy link
Contributor Author

ahl commented Jun 26, 2024

I can certainly rebase it; when do you imagine it might be reviewed? I'd rather not rebase it multiple times if I can avoid it. Thanks! I had forgotten that this PR was still out there!

@ytmimi
Copy link
Contributor

ytmimi commented Jun 26, 2024

Fair enough. To be completely honest it'll probably a while before I'm able to review this so you can hold off on rebasing until then, but I wanted to give you a head up

@ahl
Copy link
Contributor Author

ahl commented Jun 26, 2024

Sure thing. It's already been quite a while so I didn't have any expectations of imminent review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Line breaks in string literal + ascii escape code changes meaning
3 participants