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

.gitattributes: substitute CRLF only in text files #1700

Merged
merged 1 commit into from
Apr 29, 2024
Merged

.gitattributes: substitute CRLF only in text files #1700

merged 1 commit into from
Apr 29, 2024

Conversation

ydalton
Copy link
Contributor

@ydalton ydalton commented Apr 29, 2024

Changing CRLF to LF makes sense for text files, but not images, where doing so can lead to file corruption. This commit tells Git to ignore binary files and only do the substitution in plain text files.

Fixes hydephp/hyde#239.

Changing CRLF to LF makes sense for text files, but not images, where
doing so can lead to file corruption. This commit tells Git to ignore
binary files and only do the substitution in plain text files.

Fixes hydephp/hyde#239.
@caendesilva caendesilva self-requested a review April 29, 2024 16:01
Copy link
Member

@caendesilva caendesilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Thanks for the PR!

Could you confirm that this does indeed resolve issues with binary files, whilst also performing CRLF to LF normalization on all text files, including things like .md, .php, .blade.php, etc?

@caendesilva
Copy link
Member

Okay, double checked with ChatGPT and it seems like I have indeed been at fault here. Thank you so much for making this PR! I'm gonna go ahead and merge this, and I'll also go over all the gitattribute files in the organization to make sure we do this properly.

The difference lies in how Git determines whether a file should be treated as text and have its line endings normalized to LF.

  1. * text eol=lf: This rule explicitly tells Git that all files should be treated as text files, regardless of their content. It applies LF line endings to all files, irrespective of whether Git considers them as text or binary.

  2. * text=auto eol=lf: This rule tells Git to automatically detect whether a file is text or binary. If Git determines a file to be text (based on its content), it will apply LF line endings to it. If Git determines a file to be binary, it will not modify its line endings.

In summary, the difference lies in the specificity of the rule:

  • With text, all files are explicitly treated as text.
  • With text=auto, Git automatically determines whether a file is text or binary.

@caendesilva caendesilva merged commit 1cc50b1 into hydephp:master Apr 29, 2024
15 checks passed
@ydalton
Copy link
Contributor Author

ydalton commented Apr 29, 2024

Thanks for the merge. Regardless, I did testing as you said...

Okay, so I created a new project with composer create-project hyde/hyde and created a Git repository, got a PNG file, then as a control I converted _pages/index.blade.php to a CRLF file using unix2dos, and added those to Git, where I got this:

pino@my-distrobox:~/Documents/Projects/hyde$ git add .
warning: in the working copy of '_media/header.png', CRLF will be replaced by LF the next time Git touches it
warning: in the working copy of '_pages/index.blade.php', CRLF will be replaced by LF the next time Git touches it

I then changed the .gitattributes like in this commit, and added the files the same way, now with a better looking output:

pino@my-distrobox:~/Documents/Projects/hyde$ git add .
warning: in the working copy of '_pages/index.blade.php', CRLF will be replaced by LF the next time Git touche
s it

By the way, thank you for this lovely static site generator!

@caendesilva
Copy link
Member

Thank you so much for testing it!

I then changed the .gitattributes like in this commit, and added the files the same way, now with a better looking output:

pino@my-distrobox:~/Documents/Projects/hyde$ git add .
warning: in the working copy of '_pages/index.blade.php', CRLF will be replaced by LF the next time Git touche
s it

Awesome, that's just what I want. Since I do a lot of my development on a Windows machine I want to normalize text files (like Blade files for example) but I obviously don't want that to happen to binary files.

By the way, thank you for this lovely static site generator!

Thank you for the kind words and all your help debugging and patching this issue!

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.

Compiled images (on github pages) not working as expected
2 participants