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

Add lint rules to prevent CRLF and whitespace errors #2330

Closed
matrss opened this issue Apr 18, 2024 · 4 comments
Closed

Add lint rules to prevent CRLF and whitespace errors #2330

matrss opened this issue Apr 18, 2024 · 4 comments
Labels
bug Something isn't working tests
Milestone

Comments

@matrss
Copy link
Collaborator

matrss commented Apr 18, 2024

          Do we want to introduce a linter for these kinds of things? We could setup a https://editorconfig.org/ configuration for the repository and use https://github.com/editorconfig-checker/editorconfig-checker to enforce it.

Originally posted by @matrss in #2328 (comment)

With such a setup we could enforce unix-style linebreaks, linebreaks at EOF, and other stuff.

@matrss matrss added this to the 8.3.5 milestone Apr 18, 2024
@matrss matrss added the bug Something isn't working label Apr 18, 2024
@matrss
Copy link
Collaborator Author

matrss commented Apr 18, 2024

There are some inconsistencies regarding linebreak styles in the repository as shown in #2328 (comment).

@matrss
Copy link
Collaborator Author

matrss commented Apr 19, 2024

The end_of_line setting of editorconfig might be problematic because it would enforce lf on windows as well. But git can deal with this by itself: with * text=auto in .gitattributes it will auto-detect text files and convert crlf to lf in the repository. For checkouts on windows systems one could then set core.autocrlf to get crlf endings in the working tree, while git would store everything with lf.

But I have no idea if this is the best approach or if there are some files that should always have crlf (.bat?) while some should always have lf? I wish this was standardised decades ago...

@ReimarBauer
Copy link
Member

Also on windows we need to use linefeed for our repo. There should not get any \r\n into the codebase.

The bat file is an interesting question. I think Call and goto because of labels have a limitation. But I am not sure if the labelparser has still that problem.

In the past I had also problems with numbers having crlf without a blank by reading on linux.

This was referenced Apr 22, 2024
@matrss
Copy link
Collaborator Author

matrss commented Apr 22, 2024

It turned out that editorconfig is sub-optimal for this kind of thing because 1. its end_of_line setting would force lf even on windows, where actually we just want to force lf in the repository (git can create a worktree with crlf on windows, which is fine as long as the commited files have lf) and 2. its insert_final_newline option is interpreted by editorconfig-checker to mean "at least one newline", not exactly one newline.

Instead, a bit of scripting around git can do all the checking, which I have done in #2334.

@matrss matrss changed the title Add editorconfig configuration and lint workflow Add lint rules to prevent CRLF and whitespace errors Apr 22, 2024
@matrss matrss closed this as completed May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tests
Projects
None yet
Development

No branches or pull requests

2 participants