-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix(fmt): estimate size + account for all blocks #11824
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
Conversation
does this fix 11821? |
crates/fmt/src/state/mod.rs
Outdated
size += line.trim().len(); | ||
|
||
prev_needs_space = self.config.bracket_spacing | ||
&& (line.ends_with('(') || line.ends_with('{')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that right, cannot end in "{\n" or extra space after for example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i missed a parenthesis, i wanted to do:
(self.config.bracket_spacing && (line.ends_with('(') || line.ends_with('{')))
meaning that if the config tells us that we must print a space after a bracket, we will always add either a space or a line brake, so we know size will be +1
if this closes 11821 please mark it as such |
confirmed that the fix works fine on 0x-settler and ithaca/account pending @grandizzy final review after testing on other repos like euler or uni |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Motivation
closes:
Solution
this PR introduces 2 fixes:
PR Checklist