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

Better handling of CRLF #693

Closed
asleire opened this issue Apr 30, 2024 · 2 comments · Fixed by #717
Closed

Better handling of CRLF #693

asleire opened this issue Apr 30, 2024 · 2 comments · Fixed by #717
Labels

Comments

@asleire
Copy link

asleire commented Apr 30, 2024

As discussed in Styra community Slack there is an issue with the opa-fmt rule, Windows' line endings and IDEs like VS Code that automatically changes line endings.
image

Regal shows a warning when the file contains CRLF line endings. Running opa fmt through the IDE does not fix the warning because when opa fmt outputs LF line endings the IDE automatically changes the LF line endings to CRLF line endings.

Users can usually fix this quite easily by configuring their IDE to use LF endings, but the the problem of CRLF line endings is not obvious for users to begin with. It would be nice if Regal could somehow let the user know about the issue with CRLF line endings.

I have two ideas on how this could be improved:

  1. Update the current warning message to include a note about CRLF. Maybe the message could be dynamic and only include a note about CRLF if such endings are found in the file?
  2. or a new Regal rule to warn about CRLF endings
@anderseknert
Copy link
Member

Thanks @asleire!

Thinking more about this, I think we could interecept the "will save document" event in the language server, and send back a notice (modal) to the client about disabling CRLF (or what have you) line endings if encountered in the document. We should only do that if the opa-fmt rule is enabled though, as otherwise we shouldn't interfer with whatever choices the user has made. What do you think?

As a side note — one way to avoid having editors mess with this is to use an .editorconfig file at the project root, specifically for Rego. We do this in Regal, although we share the same config for both Rego and Go (as they follow the same conventions in this regard): https://github.com/StyraInc/regal/blob/main/.editorconfig

I'm not sure VS Code uses this file without custom extensions, but GitHub and IntellJ certainly do.

@asleire
Copy link
Author

asleire commented Apr 30, 2024

I think we could interecept the "will save document" event in the language server, and send back a notice (modal) to the client about disabling CRLF

That sounds great!

As a side note — one way to avoid having editors mess with this is to use an .editorconfig file at the project root, specifically for Rego

Yes, it shouldn't be difficult in most IDEs to use LF endings

anderseknert added a commit that referenced this issue May 14, 2024
In case either `opa-fmt` or `use-rego-v1` is enabled, we'll send back
a warning to the client whenever they save a document containing CRLF
line endings. This to avoid confusion when the user tries to fix formatting
using `opa fmt`, but the editor adding back CRLF line endings after.

Also disable the nestif linter, as it's annoying.

Fixes #693

Signed-off-by: Anders Eknert <anders@styra.com>
anderseknert added a commit that referenced this issue May 14, 2024
In case either `opa-fmt` or `use-rego-v1` is enabled, we'll send back
a warning to the client whenever they save a document containing CRLF
line endings. This to avoid confusion when the user tries to fix formatting
using `opa fmt`, but the editor adding back CRLF line endings after.

Also disable the nestif linter, as it's annoying.

Fixes #693

Signed-off-by: Anders Eknert <anders@styra.com>
anderseknert added a commit that referenced this issue May 14, 2024
In case either `opa-fmt` or `use-rego-v1` is enabled, we'll send back
a warning to the client whenever they save a document containing CRLF
line endings. This to avoid confusion when the user tries to fix formatting
using `opa fmt`, but the editor adding back CRLF line endings after.

Also disable the nestif linter, as it's annoying.

Fixes #693

Signed-off-by: Anders Eknert <anders@styra.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants