cheshirekow / cmake_format

Source code formatter for cmake listfiles.
GNU General Public License v3.0
954 stars 105 forks source link

Set default line-ending option to auto #203

Closed okainov closed 4 years ago

okainov commented 4 years ago

I have my Windows environment with some cmake files.

After running cmake-format (I do it inside pre-commit) I get dozens of line ending changes


warning: LF will be replaced by CRLF in sdk/CMakeLists.txt.
The file will have its original line endings in your working directory

That's very unfortunate because that seems to be the only option I need to customize from the default config, IMO it would be much more useful to have auto by default (as Git does)

cheshirekow commented 4 years ago

The default line ending option is intentionally not auto. One of the goals of cmake-format is to help projects maintain a consistent format for all their listfiles. auto will allow a mix of some files with one line ending and some files with a different line ending. In my opinion, it's better to be explicit and just set what you want in the config so that everyone working on that project ends up with the same line endings.

okainov commented 4 years ago

But I think majority of git users have git lines endings policy as "auto" (especially on Windows where it's default option). And if project is cross-platform, then cmake-format can (and eventually will) be run by developers on both Windows and Linux

cheshirekow commented 4 years ago

But I think majority of git users have git lines endings policy as "auto" (especially on Windows where it's default option).

Really? What makes you say that? You're talking about core.autocrlf right? The default is false. I would expect the majority of users have it set to the default, but I wouldn't really claim one way or the other without some evidence.

I also don't see why this would support changing the default for cmake-format line endings to auto. Why do you think this should be the case?

cmake-format can (and eventually will) be run by developers on both Windows and Linux

It already is. And regardless of whether the user is on window or linux the output of the format command will be consistent according to the project configuration.

okainov commented 4 years ago

The default is false

Not sure about that. Here is some random screenshot from Git for Windows installation. First is default option.

Moreover, false is explicitly not recommended

image

okainov commented 4 years ago

I also don't see why this would support changing the default for cmake-format line endings to auto. Why do you think this should be the case?

Because Windows users have autocrlf=true so their fileset has CRLF and Linux users use whatever is Linux default and their fileset has LF endings. They both want to run the cmake-format against their fileset.

will be consistent according to the project configuration.

IF there is specific project configuration. In our team we were quite happy to use default config (== no specific configuration in the project) until now. And I think asking people to create their custom config just because of this setting (which would be more useful to be auto by default, as noted) is not the right way...

cheshirekow commented 4 years ago

Not sure about that. Here is some random screenshot from Git for Windows installation. First is default option.

I've never used a graphical installer to install git. Not sure where this is from. Is this newish? Also: https://github.com/git/git/blob/936d1b989416a95f593bf81ccae8ac62cd83f279/environment.c#L48

In our team we were quite happy to use default config (== no specific configuration in the project) until now

Then you were happy with newlines as the line ending on output. What changed?

If you choose auto line endings, you will end up with mixed line endings in your repository. That is exactly the thing we are trying to avoid here. The configuration option is there, if you really want to use it... I don't see why that should be the default.

What is the behavior of git with autocrlf=true and committing a file with newlines as line endings?

okainov commented 4 years ago

I've never used a graphical installer to install git.

I think 100% of Windows users use GUI to install git :) It was there since forever.

Then you were happy with newlines as the line ending on output. What changed?

Nope, default config is not auto, to it uses LF, and I'm getting git diff which touched all the files affected by cmake-format since cmake-format screwed up all the line endings in all cmake files.

If you choose auto line endings, you will end up with mixed line endings in your repository. That is exactly the thing we are trying to avoid here. The configuration option is there, if you really want to use it... I don't see why that should be the default.

Nope, git handles line endings by itself.

What is the behavior of git with autocrlf=true and committing a file with newlines as line endings?

As written in the description (see installer screenshot above). Commit as LF, checkout as CRLF. Quite transparent for the users, they don't worry about it at all.

cheshirekow commented 4 years ago

I think 100% of Windows users use GUI to install git :) It was there since forever.

As I said. I have never used it. So 100% minus one at least.

Nope, default config is not auto, to it uses LF, and I'm getting git diff which touched all the files affected by cmake-format since cmake-format screwed up all the line endings in all cmake files.

You said you were " quite happy to use default config". newline has always been the default. So why did you used to be happy with it, but now you are not?

Nope, git handles line endings by itself.

Well this is a matter of configuration now isn't it? You have autocrlf=true on your window system, and I have autocrlf=false on my windows system.

Commit as LF, checkout as CRLF. Quite transparent for the users, they don't worry about it at all.

Hm... Did you misread my question? If cmake-format writes out LF by defautl and you're telling me that everything is just fine then why should the default change?

okainov commented 4 years ago

You said you were " quite happy to use default config". newline has always been the default. So why did you used to be happy with it, but now you are not?

Because we were not running cmake-format on Windows at all :D And now in our team we added pre-commit so everyone can run checks earlier directly locally and then we found out that developers on Windows can not really run the same checks without custom config with just this option changed...

Hm... Did you misread my question? If cmake-format writes out LF by defautl and you're telling me that everything is just fine then why should the default change?

I'm not saying "everything is just fine", I'm saying opposite. Or, to be precise, "everything is just fine" for Linux, yes. For Windows (with default setup) no.

cheshirekow commented 4 years ago

I'm not saying "everything is just fine", I'm saying opposite. Or, to be precise, "everything is just fine" for Linux, yes. For Windows (with default setup) no.

Ok so, then, I repeat my question:

What is the behavior of git with autocrlf=true and committing a file with newlines as line endings?

And, to summarize, your argument is that cmake-format should sacrifice consistent output behavior in order to yield "no pre-stage delta generated" on the particular case of a project which:

  1. uses git
  2. on a windows system
  3. in the presence of a .gitconfig or .git/config containing autocrlf=true
  4. and in the absence of a .gitattributes which overrides it for cmake files

And by sacrificing consistent output I mean for any project which:

  1. doesn't use git
  2. isn't on windows
  3. is on windows but lacks a .gitconfig or .git/config
  4. is on windows, has a .gitconfig or a .git/config but autocrlf is overridden (at least for cmake-files) via `.gitattributes

All just to save your windows users from writing a two-line file somewhere above their repository checkout: .cmake-format.py

with section("format"):
  line_endings = "windows"

Or to save your project from having to including a .cmake-format.py

Is that an accurate statement of your position?

okainov commented 4 years ago

Is that an accurate statement of your position?

Not really.

And, to summarize, your argument is that cmake-format should sacrifice default consistent Linux-specific output behavior in order to yield "no pre-stage delta generated" on the particular case of a project which:

uses git on a windows system in the presence of a .gitconfig or .git/config containing autocrlf=true default git OS-specific settings and in the absence of a .gitattributes which overrides it for cmake files

.

All just to save your windows users from writing a two-line file somewhere above their repository checkout: .cmake-format.py

with section("format"): line_endings = "windowsauto"

Feel free to keep it as it is if you don't want to change default consistent Linux-specific behavior.

The suggestion was to make the usage of the tool easier for, in my opinion, vast majority of users on Windows. Linux people wouldn't even notice this change and Windows people would get default behavior which is consistent with the other tools, including git, Github and some of the text editors which use OS native line endings.

I don't really see the point in continuing this further.

cheshirekow commented 4 years ago

~on a windows system~

well you said this is only an issue on a windows system yes? or did I miss something here?

~autocrlf=true~ default git OS-specific settings

Please check git config --list --show-origin | grep autocrlf and see for yourself where it comes from.

All just to save your ~windows~ users

Well, you said your non-windows users don't need to anything already right? If all your users need a config, then put it in the repo and no one has to do anything.

line_endings = "~windows~auto"

Based on the discussion so far, I don't think you want auto. Auto will replace all line endings in the file with whatever the most prominent line ending originally in that file. That can be newline even on windows and it can be crlf even on linux, mac, and unix. Sounds like what you want is crlf on windows within a git repository with autocrlf=true and newline everywhere else (including windows within a git repository with autocrlf=false).

The suggestion was to make the usage of the tool easier for, in my opinion, vast majority of users on Windows

Evidence? I'm skeptical as this does not match my experience. Whether I'm using windows or not, I'd rather not have the output line endings depend on the input line endings. I want the output line endings to be the same thing regardless of input.

Linux people wouldn't even notice this change

They will if they try to format a listfile which starts off with crlf as the line ending.

I don't really see the point in continuing this further.

I am happy to conclude the discussion.

okainov commented 4 years ago

Please check git config --list --show-origin | grep autocrlf and see for yourself where it comes from.

file:"C:\\ProgramData/Git/config"       core.autocrlf=true
cheshirekow commented 4 years ago

Which is to say:

in the presence of a .gitconfig or .git/config containing autocrlf=true

CraigHutchinson commented 1 year ago

👍 for this issue. I Am not clear what the resolution is seeing this one as closed already?

I am trying to contribute to CPM that uses cmake-format and as I am a windows developer EVERY source file and EVERY line therein is being modified when fix is applied! i.e. Code that is fine is being modified by cmake-format!