Skip to content

Handle CRLF and mixed-ending files #1162

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
wants to merge 4 commits into from

Conversation

edobez
Copy link

@edobez edobez commented May 7, 2025

Changes for end-of-file-fixer.

Refactor the handling of line endings to support files with CRLF and mixed endings.
In the first case, it will add a CRLF for the last line.
In the second case, it will default to LF for the last line.

edobez added 4 commits May 7, 2025 08:44
Case is for file using CRLF endings
Added case for which the file has mixed line endings.
In this case, default into using LF
for end of file line.
@edobez
Copy link
Author

edobez commented May 7, 2025

Refers to discussion in #1078.

@edobez edobez marked this pull request as ready for review May 7, 2025 09:06
@asottile
Copy link
Member

asottile commented May 7, 2025

reading the whole file into memory defeats the purpose of this hook

@asottile asottile closed this May 7, 2025
@edobez
Copy link
Author

edobez commented May 7, 2025

File could also be read in chunks. Is the problem reading the whole file in memory or traversing the whole file?

A possible solution that doesn't involve reading the whole file (in one way or another) is to detect the ending from the first line and assume that is the "selected" ending for the file. Of course this wouldn't catch the case of mixed endings, but might be good enough.

@asottile
Copy link
Member

asottile commented May 7, 2025

both

@edobez
Copy link
Author

edobez commented May 7, 2025

Ok, so what about the solution of not caring about mixed line endings?

@edobez
Copy link
Author

edobez commented May 7, 2025

I pushed that solution into my branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants