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

Improve formatting #16113

Closed
wants to merge 6 commits into from
Closed

Conversation

BenjaminBrienen
Copy link
Contributor

@BenjaminBrienen BenjaminBrienen commented Oct 26, 2024

Objective

Format the code optimally. This helps avoid conflicts and unnecessary changes.

For example, when trying to use Find & Replace to make code changes, formatting everything on 1 line makes it simpler. After the Find & Replace operations, formatting can be returned back to normal. This is what I did in #16112.

Solution

  1. In rustfmt.toml set max_width = 9999.
  2. Run cargo fmt --all.
  3. Remove max_width. (sets it back to 100)
  4. Run cargo fmt --all.

I consider this to be a workaround to a bug in rustfmt where the resulting formatted code depends on how it was formatted beforehand.

Testing

CI

@mockersf
Copy link
Member

I would prefer this to be blocked until after the 0.15 is released, as it has high chances of conflicts during cherry picks.

@BenjaminBrienen BenjaminBrienen self-assigned this Oct 26, 2024
@BenjaminBrienen BenjaminBrienen added D-Trivial Nice and easy! A great choice to get started with Bevy S-Blocked This cannot move forward until something else changes A-Cross-Cutting Impacts the entire engine S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Code-Quality A section of code that is hard to understand or change labels Oct 26, 2024
@IceSentry
Copy link
Contributor

IceSentry commented Oct 26, 2024

This turns lines that were already above 100 to be even longer. I don't see this as a win at all. Especially the one with long strings that put all the parameters on the same already way too long line.

I'm pretty sure a bunch of those are bugs in rustfmt where it just gives up when lines are super long because of a string.

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Oct 26, 2024
@BenjaminBrienen BenjaminBrienen added S-Wontfix This issue is the result of a deliberate design decision, and will not be fixed and removed S-Blocked This cannot move forward until something else changes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 1, 2025
@BenjaminBrienen BenjaminBrienen deleted the fmt branch January 1, 2025 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Cross-Cutting Impacts the entire engine C-Code-Quality A section of code that is hard to understand or change D-Trivial Nice and easy! A great choice to get started with Bevy S-Wontfix This issue is the result of a deliberate design decision, and will not be fixed X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants