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

Don't normalize strings in the CLI #127

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DavisVaughan
Copy link
Collaborator

Closes #90
Closes #123

Follow up to #78

  • The CLI now never normalizes line endings, allowing --check to work correctly, and allowing us to take advantage of an optimization where we detect that no changes occurred during the formatting
  • The LSP continues to always normalizes line endings to Unix endings. Since everything is Unix line endings there, we could add the optimization there too (and pre-emptively return None in the response to the format request if we detect no changes, rather than running an expensive diff algorithm)

As mentioned in #90, our parser and Biome's formatter are happy with alternative line endings when they appear in the trivia, but if non-unix line endings appear in a token then the formatter panics. This happens in multiline strings, so we now normalize strings efficiently using a Cow::Borrowed when nothing changed.

The way this all flows through biome is (as implemented by rome/tools#1672):

  • We send a parse tree with CRLF line endings into the formatter
  • The formatter normalizes all trivia to Unix line endings
  • We normalize all tokens to Unix line endings (only multiline strings have this issue)
  • At Print time, the formatter turns a Unix line ending into the user requested LineEnding

The last step there is why the Printer doesn't allow any non-Unix line endings internally. It really just looks for \n at print time to decide when to apply LineEnding, so \r\n would make it behave incorrectly.

…alize in the CLI

Allowing us to actually take advantage of the `Unchanged` optimization with CRLF endings, and correctly handle `--check` too
To prove we can parse these line endings, and to prove that the CRLF ends up in the `RStringValue`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CRLF makes --check report a change while there is nothing to modify Dont normalize line endings in the cli
1 participant