Skip to content

Conversation

0xrusowsky
Copy link
Contributor

@0xrusowsky 0xrusowsky commented Sep 25, 2025

Motivation

closes:

Solution

this PR introduces 2 fixes:

  • when estimating the size of a snippet, only account for line breaks that are preceeded by a line that ends with a semicolon.
  • track block depth so that, when estimating the space left, we can account for all of them rather than just contracts and funcs

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@onbjerg
Copy link
Contributor

onbjerg commented Sep 25, 2025

does this fix 11821?

size += line.trim().len();

prev_needs_space = self.config.bracket_spacing
&& (line.ends_with('(') || line.ends_with('{'))
Copy link
Collaborator

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?

Copy link
Contributor Author

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

@onbjerg
Copy link
Contributor

onbjerg commented Sep 25, 2025

if this closes 11821 please mark it as such

@0xrusowsky
Copy link
Contributor Author

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

@0xrusowsky 0xrusowsky enabled auto-merge (squash) September 26, 2025 08:01
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

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

lgtm

@0xrusowsky 0xrusowsky merged commit 6c381be into master Sep 26, 2025
16 checks passed
@0xrusowsky 0xrusowsky deleted the rusowsky/fmt-estimate-size branch September 26, 2025 08:27
@github-project-automation github-project-automation bot moved this to Done in Foundry Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

bug(fmt): subsequent runs format same code with different style

3 participants