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.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

PR 2730, "Fix" line-endings

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

Re: PR 2730, "Fix" line-endings

Post 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: )
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: PR 2730, "Fix" line-endings

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

Re: PR 2730, "Fix" line-endings

Post by Kunda1 »

Related issue #1352
Alone you go faster. Together we go farther
Please mark thread [Solved]
Want to contribute back to FC? Checkout:
'good first issues' | Open TODOs and FIXMEs | How to Help FreeCAD | How to report Bugs
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: PR 2730, "Fix" line-endings

Post by DeepSOIC »

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

Re: PR 2730, "Fix" line-endings

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

Re: PR 2730, "Fix" line-endings

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

Re: PR 2730, "Fix" line-endings

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

Re: PR 2730, "Fix" line-endings

Post 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.
Always add the important information to your posts if you need help. Also see Tutorials and Video tutorials.
To support the documentation effort, and code development, your donation is appreciated: liberapay.com/FreeCAD.
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

Re: PR 2730, "Fix" line-endings

Post 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.
Post Reply