Update .gitattributes to prefer LF

[seanbudd]

Link to issue number:

Part of #12387

Summary of the issue:

The NVDA code base uses a mix of CRLF and LF style line endings, with the preference being CRLF.
LF style endings have been introduced prior to lint checking, and can causes issues when moving code or making changes.
Many IDEs will normalize the line endings when editing files, causing larger than intended diffs of files with mixed line endings.
Instead, we should normalize all line endings of text files to LF and ensure that all files are committed with LF line endings.

Specific files should be excluded from normalization such as:

• files used by translators, as we are uncertain of the side-effects here
• binary files

git allows independent configurations between line endings used when a developer checkouts out code locally, and when code is committed to a repository.
IDEs are often better suited for different line endings, with LF line endings becoming more and more standardised.
However, NVDA is Windows based development, where CRLF line ending usage is commonly default or compulsory.
Developers can checkout code locally, using whatever line endings they prefer or their IDE requires, using git config.core autocrlf.

We can configure what line endings we commit to the repository with .gitattributes.
This is set on a repository level.
As what line endings we commit with is agnostic to what is checked out, the decision is based on what works best for the repository.
Unix style line endings usage and support continues to increase on Windows and LF files.
LF tends to be better supported by Unix style developer tools such as git.
LF uses less data than CRLF.

Additional information: https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings

Description of how this pull request fixes the issue:

Fixing this is a 3 step process, this PR emulates the 3 steps, but only one commit should be merged at a time.

1. The first commit updates .gitattributes so that any new files committed will use LF style endings where appropriate.
2. Then, no PRs should be merged until a commit which normalizes the line endings is committed to master.
   Otherwise, new PRs will have large diffs due to line ending changes.

   git pull origin/master # to get .gitattributes
   git add --renormalize .
   git commit -m "fix line endings"

3. Then, the third commit can be committed. This updates the .gitignorerevs file so that the normalization commit can be ignored by developers using git blame.

Specifically this PR:

• Updates .gitattributes file, with eol=lf and normalization patterns for each file type NVDA uses:
  • -text or binary for no normalization
  • text to change line endings to lf when committing code.
• Ensures git add --renormalize . sets the correct line endings for the entire project.
• Adds a .gitignorerevs file
• Updates documentation
  • Contributing wiki and other wikis may need to be updated.

Testing strategy:

Setup normalize-line-endings as follows to perform this testing.

git checkout normalize-line-endings
git reset origin/normalize-line-endings --hard
git add --renormalize .
git commit -m "fix line endings"

Update .gitignorerevs with the latest commit.

Compare the diff while ignoring eol line changes

git diff --ignore-cr-at-eol origin/master normalize-line-endings

Check the blame of a file from this branch and confirm the line ending changes show up in the blame.

Then check the blame of a file:

git blame source/core.py normalize-line-endings

Test that .gitignorerevs works and ignores the line ending changes.

git blame --ignore-revs-file .gitignorerevs source/core.py normalize-line-endings

Test that setting .gitignorerevs works.

This should be recommended to devs.

git config --local blame.ignoreRevsFile .gitignorerevs
# should ignore revs
git blame source/core.py normalize-line-endings
# should not ignore revs
git blame --ignore-revs-file "" source/core.py normalize-line-endings

Test that committing a text file, not used by the translation system, with CRLF line endings is not possible.

This can be done by adding or changing a file.

Test that the resulting translation .pot files generated do not change after nomalization.

git checkout master
scons pot
git checkout normalize-line-endings
scons pot

Note: Not git diff, as these files are ignored. Update the commits within the file names if required.

diff output/nvda_snapshot_source-master-6b4cf45.pot output/nvda_snapshot_source-normalize-line-endings-31dee0a.pot

cat -A displays CR as ^M. This will ensure the diff tool picks up on any CRLF changes.

cat -A output/nvda_snapshot_source-master-6b4cf45.pot > master-with-explicit-line-endings.pot
cat -A output/nvda_snapshot_source-normalize-line-endings-31dee0a.pot > normalized-with-explicit-line-endings.pot
diff master-with-explicit-line-endings.pot normalized-with-explicit-line-endings.pot

Known issues with pull request:

None

Change log entries:

N/A

Code Review Checklist:

• Pull Request description is up to date.
• Unit tests.
• System (end to end) tests.
• Manual testing.
• User Documentation.
• Change log entry.
• Context sensitive help for GUI changes.
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers

[AppVeyorBot]

• PASS: Translation comments check.
• FAIL: Lint check. See test results for more information.
• PASS: System tests.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/kpnphdfb8956rt4u/artifacts/output/nvda_snapshot_pr12816-23698,cbce460f.exe
• PASS: Unit tests.

See test results for failed build of commit cbce460fa3

[LeonarddeR]

Git for Windows has a feature to fiddle with line endings at checkout and commit time I believe. Won't this conflict with that?

[seanbudd]

Git for Windows has a feature to fiddle with line endings at checkout and commit time I believe. Won't this conflict with that?

This should remain compatible with core.autocrlf. See the documentation changes to devDocs/codingStandards.md.

See also: https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings

[CyrilleB79]

Did you update the readme.md with the recommanded configuration? Sorry, I've not been able to find it in a such big number of modified files because I do not know how to view a specific modified file in GitHub.

[seanbudd]

Did you update the readme.md with the recommanded configuration?

There's no real recommended configuration for config.core autocrlf - the decision on what value to use is up to the developer, a preference which might be influenced by IDE support. The .gitattrbutes file should ensure line endings are committed properly, and config.core autocrlf configures how code is checked out locally. More information.
I've updated devDocs/codingStandards.md to reflect this, it's not mentioned in the readme currently.

I plan on adding git config --local blame.ignoreRevsFile .gitignorerevs to the Contributing wiki page, as that's where the git setup is mentioned. I'm not aware of any other good places to mention it.

Sorry, I've not been able to find it in a such big number of modified files because I do not know how to view a specific modified file in GitHub.

Yes this PR is a proof of concept on the strategy. I think each commit/step should be merged as separate PRs. The last and the first commits of this PR are the ones worth manually checking. Hiding whitespace changes via the diff settings makes browsing this whole diff a bit more manageable.

[seanbudd]

@feerrenrut @michaelDCurran Any ideas on how we can confirm that normalizing the line endings won't affect translators?

Otherwise, this PR is ready to be reviewed then merged with the strategy outlined in the PR description.

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/eh757531vkf0poo6/artifacts/output/nvda_snapshot_pr12816-23801,eb6e6aa1.exe

See test results for failed build of commit eb6e6aa1aa

[seanbudd]

Thanks @michaelDCurran - I've updated the testing steps and confirmed that the pot file is unchanged (other than the metadata eg commit info and generation time).

[feerrenrut]

@seanbudd When you do that diff of the pot files, ensure that the line endings were considered and not ignored by the diff tool. It may be more certain to inspect an entry in the file manually with tail file.pot | cat -A.

[feerrenrut]

I hope you don't mind I've updated the description to add a numbered lists for the stages of the change. It currently talks about 3 commits in this PR which is a little confusing.

How will the PR's be updated to resolve the diff? Each PR will have to have normalize run on it and a new commit pushed, right?

[seanbudd]

@feerrenrut

When you do that diff of the pot files, ensure that the line endings were considered and not ignored by the diff tool. It may be more certain to inspect an entry in the file manually with tail file.pot | cat -A.

I've confirmed this and updated the testing steps.

I hope you don't mind I've updated the description to add a numbered lists for the stages of the change. It currently talks about 3 commits in this PR which is a little confusing.

Thanks, this is much easier to read.

How will the PR's be updated to resolve the diff? Each PR will have to have normalize run on it and a new commit pushed, right?

I've created #12864 to investigate and discuss this. In short - yes.