search

DataObjects-NET / dataobjects-net

https://dataobjects.net
MIT License
61 stars 25 forks source link

remove `Change end_of_line` rule in `.editorconfig` to avoid confusion with `CRLF` <-> `LF` conversions on Linux dev environment #319

Closed SergeiPavlov closed 1 year ago

SergeiPavlov commented 1 year ago

Source code in the repository actually has LF ending No need to force VS insert CRLF, and then convert it by git during commits

alex-kulakov commented 1 year ago

I believe it is operating system dependent. I'm on Windows and when I open various files in Notepad++, which is capable of showing line endings, and I see CRLF all across the files and even before we switched to git we had CRLF endings in actual files.

As I remember we changed git settings to have auto endings to be more friendly to non-windows operating systems.

So no, no switching of end of line globally.

SergeiPavlov commented 1 year ago

Git settings are individual Microsoft itself switched to LF in dotnet repo Any modern text editor on Windows supports LF-only mode

BTW we had a lot of pain until forced LF endings.

alex-kulakov commented 1 year ago

1) This PR doesn't solve the issue. I assume you don't understand this. It might solve issue for you personally, but I shouldn't allow preferences of one person to become a new standard for all.

2) I'm happy for you and your team that you have found the ideal setting for YOUR project.

3) Microsoft didn't switch to LF, they don't declare eol/end_of_line for C# source code. I have checked following repos and their gitattributes and editorconfig files:

When I clone dotnet/aspnetcore repo and went through some files I see CRLF everywhere. For example, src/Mvc/Mvc.Core/src/ForbidResult.cs which was edited within few weeks has no line ended with LF on my machine. If they had switched to LF I would've seen at least some LFs.

I also cloned dotnet/sdk. There is a file with path sdk/src/Tests/Microsoft.DotNet.ApiCompat.Tests/RegexStringTransformerTests.cs which was updated 2 weeks ago. No LFs found as well, all endings are CRLF.

I also have checked the history of editorconfig files to figure out whether they had some sort of ending and then removed it and they didn't.

When you, on your Unix-based PC clone the repos above you will have LF ending in files across the board. This made you think that they have LF as default setting, but they don't. Same happened with this repo, you thought that we had LF endings because GIT did its trick.

According to current settings of GIT of this repo, windows clients will always have CRLF and Unix clients will always have files with LF. Now I work with files with native endings but you don't. If we changed ending to LF then I would suffer from this (along with all windows git clients) and you wouldn't. So, by this PR you don't solve the issue, you just move pain from your neck to mine, that's it. Why should I allow this happen? Timewise I work with the project more. All our infrastructure is windows-based, so I should take my chances of having something going wrong and possibly create some issues for me to resolve, right? No. I have to think carefully because for you it is one-line change and for me it is possibly several days of overcoming some problems which could appear.

The proof of my point about operating system based endings is Npgsql repo. They have the same "* text=auto" setting for git as we have and "end_of_line = LF" in editorconfig. All files of the Npgsql repo in file system on my PC contain CRLF because of GIT and when I edit some file then the file ends up with mixed endings (both LF and CRLF).

Potentially, removing of explicit end_of_line declaration may help, in theory, because we will let git to convert them and both our code editors will work with native endings. But this theory should be tested thoroughly on both types of operating system.

I strongly recommend you discuss any change of settings of GIT, formatting (or any other setting that could potentially harm other people working with the repo) before making them, otherwise I will reject such pull-requests without having to explain the reason as I have to do now. Because it took a lot of time to prove my point for you, several hours, actually, to search, collect and compose information in form of this detailed answer.

SergeiPavlov commented 1 year ago

For example, src/Mvc/Mvc.Core/src/ForbidResult.cs which was edited within few weeks has no line ended with LF on my machine. If they had switched to LF I would've seen at least some LFs.

Probably your Git client converts LF->CRLF on checkout (Windows clients do it, but Linux clients provide files as is) If you download https://github.com/dotnet/aspnetcore/blob/main/src/Mvc/Mvc.Core/src/ForbidResult.cs directly by browser you will see that it has no CR chars as well as other dotnet files

BTW .cs files in this DO repo also have LF ends-of-line So it is unclear for me why end_of_line = crlf was used at all

you thought that we had LF endings because GIT did its trick.

GIT did its trick ONLY for windows, and REAL ending is LF.

will let git to convert them and both our code editors

I think it would be better to disable Git to do conversion by autocrlf = input or = false in personal settings and remove .gitattributes, because any implicit conversion increases complexity. I dont see any pros to have CRLF on Windows machines, it is a heritage of DEC's VT52 terminals from 1970s

And even having end_of_line = crlf is better than absense of this setting

alex-kulakov commented 1 year ago

BTW .cs files in this DO repo also have LF ends-of-line So it is unclear for me why end_of_line = crlf was used at all

It was done so because before it became open-source it was private project when everyone worked on windows so all the sources were with CRLF endings. This setting kept us, actual team, from having possible troubles (if they appear, we all know that software is sometimes unpredictable in the issues it delivers when it is important to work properly).

GIT did its trick ONLY for windows, and REAL ending is LF.

Well, I didn't know that it stores LF, obviously it uses something as end-of-line symbol, it could be some replacer, easily. Now I know, thanks to you.

I think it would be better to disable Git to do conversion by...

No, it will lead to all files' modification. I have experienced it already. Don't want it again. It works now.

I think we'll be just fine if we remove end_of_line declaration from editorconfig. I played with Linux few days to check it. But, in my opinion, it should be done in branch 6.0 and merged to all newer branches, not only in master.

SergeiPavlov commented 1 year ago

No, it will lead to all files' modification. I have experienced it already. Don't want it again. It works now.

No need for all files conversion, they are already LF Just every developer can use his own personal autocrlf settings in .git/config.

Right now .gitattributes has no effect (at least for me). I still see LF-ended lines even on Windows machine

alex-kulakov commented 1 year ago

The point of git attributes and editor config is to unify all the developers and contributors by settings. This is reasonable because it prevents personal settings having affect on the project.

I agree that you can remove the line about end_of_file from editorconfig file by this PR. Other branches are on me. This is all I can allow.

alex-kulakov commented 1 year ago

Or I can pass the change from 6.0 all the way up to the master.

SergeiPavlov commented 1 year ago

I agree that you can remove the line about end_of_file from editorconfig file by this PR. Other branches are on me. This is all I can allow.

Removed the rule