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

fix line-endings conflict between prettier and git #1015

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

Vankog
Copy link
Contributor

@Vankog Vankog commented Nov 5, 2024

The git attribute normalizes line endings for git checkout and pushing, depending on the user's OS.
See https://git-scm.com/docs/gitattributes#_end_of_line_conversion

The prettier config does the same, so the working directory is not polluted with pseudo git-changes
when running Prettier.

This is checked on a Windows machine.
Please double check on a Unix or Mac.

* text=auto

*.sh text eol=lf
*.bat text eol=crlf
Copy link
Contributor Author

@Vankog Vankog Nov 5, 2024

Choose a reason for hiding this comment

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

These two lines are technically not necessary in this project.
However, I'd like to always add them nonetheless, as it prevents issues on WIndows machines with any possible future scripts. (e.g. with maven/gradle wrapper etc.)

Copy link
Owner

Choose a reason for hiding this comment

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

Seems reasonable

.gitattributes Outdated
@@ -0,0 +1,4 @@
* text=auto
Copy link
Owner

Choose a reason for hiding this comment

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

Back when I still used Windows, I used to add this to my git config: git config --global core.autocrlf true
Does that do the same thing?
(Except for the fact that git config pertains to my local machine and this file pertains to this repository)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, they are not quite the same. They work in tandem, though.
According to the docs: https://git-scm.com/docs/gitattributes#_end_of_line_conversion

While Git normally leaves file contents alone, it can be configured to normalize line endings to LF in the repository and, optionally, to convert them to CRLF when files are checked out.

If you simply want to have CRLF line endings in your working directory regardless of the repository you are working with, you can set the config variable "core.autocrlf" without using any attributes.

[core]
	autocrlf = true

This does not force normalization of text files, but does ensure that text files that you introduce to the repository have their line endings normalized to LF when they are added, and that files that are already normalized in the repository stay normalized.

If you want to ensure that text files that any contributor introduces to the repository have their line endings normalized, you can set the text attribute to "auto" for all files.

*	text=auto

So, the attributes are basically a more granular EOL-setting for all users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See rebased version.
I experimented with it on my machine and it works as expected.
At least for newly checked-out repos all files are LF coming in. For some reason git add --renormalize . didn't really work. Not sure if this is a git bug or whatever. But in the end it works fine.

Copy link
Owner

Choose a reason for hiding this comment

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

I don't think there are any Windows users who are keeping a long-running checkout of this project, so we're probably fine :)

.prettierrc.yml Outdated
@@ -1,3 +1,4 @@
tabWidth: 4
printWidth: 100
trailingComma: "none"
endOfLine: "auto"
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not 100% sure this does the same as the git option. According to the git docs, text=auto should make sure that only LF's go into the repository, while according to Prettier's docs it maintains existing line endings, but might change them if in a file with mixed endings. Wouldn't it be better to set it to "lf" here?

Also, if Git makes sure that only lf enters the repository, does it still matter what Prettier does?
I'm also asking this because at some point soon-ish, I intend to replace Prettier, and I don't know yet how its replacement will behave in this specific situation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, Prettier's auto mode is not the same. Thanks for bringing this to my attention. I read the doc, but skipped this part.
To be honest, for Windows systems, it is quite a pain that Prettier is enforcing the line endings. This should be the responsibility of the git attributes.
For this project we can probably get away with the suggestion of Prettier:

If you want to make sure that your entire git repository only contains Linux-style line endings in files covered by Prettier:

  1. Ensure Prettier’s endOfLine option is set to lf (this is a default value since v2.0.0)
    ...
  2. Add * text=auto eol=lf to the repo’s .gitattributes file. You may need to ask Windows users to re-clone your repo after this change to ensure git has not converted LF to CRLF on checkout.
  3. All modern text editors in all operating systems are able to correctly display line endings when \n (LF) is used. However, old versions of Notepad for Windows will visually squash such lines into one as they can only deal with \r\n (CRLF).

There are project's where the line endings must match properly, especially with script files. But I think we don't have any issues here.
I'll adapt the PR accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See other thread.

The git attribute normalizes line endings for checkout and pushing depending on
the user's OS.
See https://git-scm.com/docs/gitattributes#_end_of_line_conversion

The prettier config does the same, so the working directory is not polluted when
running prettier.
https://prettier.io/docs/en/options#end-of-line
@Vankog Vankog force-pushed the auto-line_endings branch from 13b7826 to ccab821 Compare November 6, 2024 10:44
@jqno jqno merged commit a5c9288 into jqno:main Nov 6, 2024
8 checks passed
@jqno
Copy link
Owner

jqno commented Nov 6, 2024

Thanks for looking into all of this and making this repo more accessible for Windows users!

@Vankog Vankog deleted the auto-line_endings branch November 6, 2024 12:28
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.

2 participants