PR 2730, "Fix" line-endings

Post here if you have re-based and finalised code to integrate into master, which was discussed, agreed to and tested in other forums. You can also submit your PR directly on github.
ezzieyguywuf
Posts: 637
Joined: Tue May 19, 2015 1:11 am

PR 2730, "Fix" line-endings

Postby ezzieyguywuf » Fri Nov 22, 2019 8:00 pm

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.
mlampert
Posts: 1443
Joined: Fri Sep 16, 2016 9:28 pm

Re: PR 2730, "Fix" line-endings

Postby mlampert » Fri Nov 22, 2019 8:46 pm

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: )
User avatar
DeepSOIC
Posts: 7245
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: PR 2730, "Fix" line-endings

Postby DeepSOIC » Fri Nov 22, 2019 9:41 pm

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)
User avatar
Kunda1
Posts: 6223
Joined: Thu Jan 05, 2017 9:03 pm

Re: PR 2730, "Fix" line-endings

Postby Kunda1 » Fri Nov 22, 2019 9:56 pm

Related issue #1352
Want to contribute back to FC? Checkout:
#lowhangingfruit | Use the Source, Luke. | How to Help FreeCAD | How to report FC bugs and features
User avatar
DeepSOIC
Posts: 7245
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: PR 2730, "Fix" line-endings

Postby DeepSOIC » Fri Nov 22, 2019 10:15 pm

Kunda1 wrote:
Fri Nov 22, 2019 9:56 pm
Related issue #1352
Nice link btw, it has a good discussion
ezzieyguywuf
Posts: 637
Joined: Tue May 19, 2015 1:11 am

Re: PR 2730, "Fix" line-endings

Postby ezzieyguywuf » Sat Nov 23, 2019 1:09 am

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
ezzieyguywuf
Posts: 637
Joined: Tue May 19, 2015 1:11 am

Re: PR 2730, "Fix" line-endings

Postby ezzieyguywuf » Sat Nov 23, 2019 1:20 am

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.
triplus
Posts: 8888
Joined: Mon Dec 12, 2011 4:45 pm

Re: PR 2730, "Fix" line-endings

Postby triplus » 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?

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.
vocx
Posts: 2450
Joined: Thu Oct 18, 2018 9:18 pm

Re: PR 2730, "Fix" line-endings

Postby vocx » Sun Nov 24, 2019 4:48 pm

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.
To support the documentation effort, and code development, your donation is appreciated: paypal.
ezzieyguywuf
Posts: 637
Joined: Tue May 19, 2015 1:11 am

Re: PR 2730, "Fix" line-endings

Postby ezzieyguywuf » Sun Nov 24, 2019 6:30 pm

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.