Page 1 of 5

PR 2730, "Fix" line-endings

Posted: Fri Nov 22, 2019 8:00 pm
by ezzieyguywuf
I was recently messing around in the FreeCAD code-base and noticed that when I open/closed a file using python -- without making any changes -- a `git diff` showed the entire file as being changed.

This got me to thinking, and after some digging I realized that this is because python had changed the line-endings from windows-style to unix-style.

This github page does a good job describing some best-practices for managing line-endings in git.

This pull-request is intended to reset the existing code-base such that future PR's will result in more readable and manageable diffs, as well as easier-to-manage merges.

Re: PR 2730, "Fix" line-endings

Posted: Fri Nov 22, 2019 8:46 pm
by mlampert
I'm all for it - although it would be great if you could provide links to the files you added and/or manually changed. I don't think anybody's gonna review 2705 files (incidentally almost matches the PR number :lol: )

Re: PR 2730, "Fix" line-endings

Posted: Fri Nov 22, 2019 9:41 pm
by DeepSOIC
Qt creator seems to have a habit of messing with line endings. Although, for some reason, I don't see them in diffs - GitHub app apparently hides them from me. But I do see them in command-line git, and that is a little annoying (though again, no big deal, usually I have to just git reset --hard sometimes)

Re: PR 2730, "Fix" line-endings

Posted: Fri Nov 22, 2019 9:56 pm
by Kunda1
Related issue #1352

Re: PR 2730, "Fix" line-endings

Posted: Fri Nov 22, 2019 10:15 pm
by DeepSOIC
Kunda1 wrote:
Fri Nov 22, 2019 9:56 pm
Related issue #1352
Nice link btw, it has a good discussion

Re: PR 2730, "Fix" line-endings

Posted: Sat Nov 23, 2019 1:09 am
by ezzieyguywuf
I think that if we're going to do a massive line-ending-change (my vote is to not to), it would be best to do it using something like filter-branch that rewrites history so we don't have a commit that touches every line in the project.
This is an extremely good point. I will take some time to figure out if it is possible to make these changes while preserving the git history.

if I can do it, it will mean careful coordination with contributors, as it will necessitate a force push to github.

Finally, regarding maintenance: wouldn't any pull requests with the wrong line endings show a "dirty" diff? if so, it would be a simple matter for the maintainer merging the code to have the contributor rework their pull request with the appropriate line endings.

further, as mentioned in the linked bug, I can include a .gitattributes with the pr which should take care of ensuring all contributors are using the correct git settings

Re: PR 2730, "Fix" line-endings

Posted: Sat Nov 23, 2019 1:20 am
by ezzieyguywuf
sure enough, someone figured this out back in 2013.

I have no problem doing the leg work, but I would like some more input from the core contributors and maintainers in order to ensure we all agree on the best path.

Re: PR 2730, "Fix" line-endings

Posted: Sat Nov 23, 2019 8:17 pm
by triplus
Aren't most text editors smart enough, to not mess with line endings? In addition, wouldn't different line endings emerge in the future again, due to different people using different text editors on different platforms?

P.S. I guess the pros must outweigh the cons in some meaningful way, that is if the impact can't be minimized, or it makes little sense to mess with this.

Re: PR 2730, "Fix" line-endings

Posted: Sun Nov 24, 2019 4:48 pm
by vocx
triplus wrote:
Sat Nov 23, 2019 8:17 pm
Aren't most text editors smart enough, to not mess with line endings? In addition, wouldn't different line endings emerge in the future again, due to different people using different text editors on different platforms?...
Yes, most text editors are smart, but that means that if you are using Windows line endings, you will continue using them if you edit files. Also, if you copy files to start working on your code, you will duplicate the endings of those files.

I guess the intention of ezzieyguywuf is this: set the entirety of line endings to Unix once, and then every contributor who uses a smart text editor will use Unix line endings by default; and if you duplicate a file to work on something, that duplicate will also use Unix line endings, and thus the risk of introducing Windows line endings again will be minimal.

Re: PR 2730, "Fix" line-endings

Posted: Sun Nov 24, 2019 6:30 pm
by ezzieyguywuf
vocx wrote:
Sun Nov 24, 2019 4:48 pm
I guess the intention of ezzieyguywuf is this: set the entirety of line endings to Unix once, and then every contributor who uses a smart text editor will use Unix line endings by default;
yes and no.

yes the intent is to use Unix line ending across the board.

if Windows users set their git config properly (or if we include a .gitattribute file), then git will convert to windows style line ending upon checkout, and back to Unix-style when committing.

this is the best of both worlds I imagine.