Skip to content

automation: Please, check/rewrite carrige return #2678

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

Closed
Anton-Latukha opened this issue Feb 1, 2022 · 5 comments · Fixed by #2679
Closed

automation: Please, check/rewrite carrige return #2678

Anton-Latukha opened this issue Feb 1, 2022 · 5 comments · Fixed by #2679
Labels
CI Continuous integration type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@Anton-Latukha
Copy link
Collaborator

Anton-Latukha commented Feb 1, 2022

As diff in https://github.com/haskell/haskell-language-server/pull/2597/files shows - as a side-effect there - 15 files were rewritten fully (every line in them got ^M, most probably due to Windows line break insertion).

As a result, after merge that created additional challenges doing rebases or working with those files, since there are no longer diffs in the touched files.

Would be useful to prevent such diffs in the future.

@Anton-Latukha Anton-Latukha added type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. status: needs triage labels Feb 1, 2022
@Anton-Latukha Anton-Latukha changed the title CI: Check/rewrite carrige return CI: Please, check/rewrite carrige return Feb 2, 2022
@drsooch
Copy link
Collaborator

drsooch commented Feb 2, 2022

Looks like the pre-commit hook in the contributing docs has support for mixed line endings https://pre-commit.com/hooks . Not familiar myself with the tool, so not sure how easy it is to add.

@jneira
Copy link
Member

jneira commented Feb 2, 2022

I am suffering it continuously, the repo has unix line endings as default in

end_of_line = LF

I have the .editorconfig extension installed in vscode so the project uses LF by default even for new files

And i am a good boy and have core.autocrlf=input in my git config instead the default auto

So the hook formats my code correctly, changes new lines to CRLF and then the commit fail, then i revisit the file, save it again to make the editor correct line endings to LF and redo the commit

😟

//cc @bradrn

@jneira jneira added CI Continuous integration and removed status: needs triage labels Feb 2, 2022
@jneira jneira linked a pull request Feb 2, 2022 that will close this issue
@bradrn
Copy link
Contributor

bradrn commented Feb 2, 2022

Oh dear, sorry if I messed everything up! I merely ran the pre-commit hook and committed what it altered, including all the line ending changes. I also have had issues with line endings in my repos (thanks to core.autocrlf=auto), and I still am not quite comfortable with how everything works together in this regard, so I’m probably not the best person to help here.

@jneira
Copy link
Member

jneira commented Feb 2, 2022

Oh dear, sorry if I messed everything up! I merely ran the pre-commit hook and committed what it altered, including all the line ending changes. I also have had issues with line endings in my repos (thanks to core.autocrlf=auto), and I still am not quite comfortable with how everything works together in this regard, so I’m probably not the best person to help here.

Dont worry those are the little annoyances of develop in windows, we are in minority, so please, dont let me alone :-P
And the precommit hook is wrong here for sure we have to fix it

I would suggest you switch to core.autocrlf=input and install the appropiate plugin in your editor to honour .editorconfig.
It would be useful beyond the line endings thing and for other projects too. Nowadays you can stick to LF in windows as almost all editors lets you use them (but notepad :-P? not sure maybe it can now).

core.autocrlf=auto has caveats like the described ones here: #2679 (comment) and other ones which can arise from the fact your files and the repo ones are not the same

@bradrn
Copy link
Contributor

bradrn commented Feb 2, 2022

I would suggest you switch to core.autocrlf=input

Oh, believe me, I’ve tried… but eventually I gave up; the confusion it caused in my existing repos was just too great. One of these days I should try doing this again, to see if I can get any further.

and install the appropiate plugin in your editor to honour .editorconfig.

OK, I’ve installed https://github.com/editorconfig/editorconfig-emacs.

(but notepad :-P? not sure maybe it can now).

Notepad does indeed have support for LF now! It was added in a recent update.

@Anton-Latukha Anton-Latukha changed the title CI: Please, check/rewrite carrige return automation: Please, check/rewrite carrige return Feb 3, 2022
@mergify mergify bot closed this as completed in #2679 Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants