search

pinterest / ktlint

An anti-bikeshedding Kotlin linter with built-in formatter
https://pinterest.github.io/ktlint/
MIT License
6.23k stars 512 forks source link

Ktlint 1.3.* converts crlf to lf on Windows #2747

Closed mxkmn closed 3 months ago

mxkmn commented 3 months ago

Expected Behavior

Ktlint should not affect the line break settings unless the break type is specified in .editorconfig

Observed Behavior

Ktlint sets default line breaks in Unix, not in Windows, without asking. This cannot be disabled. The Difference Editor in Idea is filled with a bunch of files that contain no changes other than line breaks. The different line breaks are not sent as commits to the repository, because git stores lf data by default as it is.

The only solution is to add *.kt text eol=lf to .gitattributes and clone the repository again, but I'm not happy with that.

Steps to Reproduce

Just create a new .kt file without special .editorconfig rules on Windows (so the file will have crlf endings) and run ktlint.

Your Environment

paul-dingemans commented 3 months ago

I think that I have found the problem. Can you please help to verify whether the solution is working as I do not have a Windows computer?

The problem seems to be restricted to files in which not lint violations is found that can be autocorrected. In such files the line endings were indeed changed from CRLF to LF. In files in which a lint violation is found, and which could also be autocorrected, the line endings were not changed.

Please run Ktlint CLI latest snapshot build or ktlint-cli-1.4.0-20240723.140247-11-all.jar on your project to check that line endings are no longer changed.

*edited: update direct link to jar to refer to fat jar

mxkmn commented 3 months ago

As I understand from the description of this pull request, ktlint will still change the ending to lf in case of finding problems in the file. If this is indeed the case, this will cause confusion in file endings, they will not be standardised under default settings in projects developed on Windows machines. What are your thoughts on this?

I'm ready to test this version of the formatter if you explain whether crlf would change to cr in case the file requires formatting, but the .editorconfig doesn't specify the corresponding rule.

paul-dingemans commented 3 months ago

My expectation is that the problem only occurred in files where no Ktlint violations were autocorrected. In that case the end of line separator was not determined whilst still updating the end of line separator with the default value.

For files in which a violation was found, the behavior was not changed in Ktlint 1.3. So in this case the end of line separator was, and still is, correctly determined.

mxkmn commented 3 months ago

Good to know.

I can't figure out how I can run ktlint-cli-1.4.0-20240723.140247-11.jar, since it is compiled without -include-runtime. Could you please help me with this? I usually use kotlinter.

paul-dingemans commented 3 months ago

I can't figure out how I can run ktlint-cli-1.4.0-20240723.140247-11.jar, since it is compiled without -include-runtime. Could you please help me with this? I usually use kotlinter.

Sorry, I should have pointed you to the fat jar and to the run instructions for Windows.

mxkmn commented 3 months ago

Okay… Now there are a few problems:

paul-dingemans commented 3 months ago
  • Without specifying end_of_line in .editorconfig, files are still formatted in lf if ktlint decides to modify them;

This behavior was changed in https://github.com/pinterest/ktlint/pull/1831 (ktlint version 0.49.0). I missed the impact of the change by setting the default value to lf when the value was not set in the .editorconfig. At this moment, I cannot revert this behavior to the old situation as too many new versions have been released since then. Especially as you are the first one who identified this as an issue.

mxkmn commented 3 months ago

Although I've already added a workaround for these problems using .gitattributes, I still think it's a bug and worth fixing.

I understand that this behavior was introduced quite a while ago, but I don't think anyone is going to be frustrated by the fact that their files are no longer randomly switching from crlf to cr. The second issue you didn't mention is also an obvious bug.

gavvvr commented 3 months ago

Faced the same issue while upgrading ktlint from 1.2.1 to 1.3.1. I do format all the code, but see no changes, only different line-endings. @paul-dingemans I can confirm that 1.4.0-SNAPSHOT solves the problem. So, waiting for 1.4.0 (or 1.3.2) to upgrade to the latest ktlint 😎

But as @mxkmn has noted, the line endings of auto-fixed files get indeed transformed from CRLF to LF even without explicit end_of_line specified in .editorconfig

I cannot revert this behavior to the old situation as too many new versions have been released since then.

I don't think anyone is going to be frustrated by the fact that their files are no longer randomly switching from crlf to cr

I am not aware of any potential harm of changing line endings of Kotlin source code, so I tend to agree that most probably nobody will be frustrated if ktlint will stop converting from CRLF to LF on auto-formatting for Windows users.

It's also interesting, that if you generate a project on Windows either with gradle init of simply with Intellij's "New project..." menu, the line ending of generated sample code will be LF anyway. But as soon as you create a new file in Intellij with Alt+Insert, it will be CRLF, so Intellij it also a bit inconsistent while generating a new project.

paul-dingemans commented 2 months ago

Although I've already added a workaround for these problems using .gitattributes, I still think it's a bug and worth fixing.

I understand that this behavior was introduced quite a while ago, but I don't think anyone is going to be frustrated by the fact that their files are no longer randomly switching from crlf to cr.

It will be a problem for projects that have started using ktlint with ktlint 0.49 or later. They might not have set the property explicitly, and be depending on the lf default which is set by ktlint when no explicit value is set. Reverting the behavior to version 0.49 would result in a breaking change for them. I prioritize users on the more recent versions of ktlint above users who skip (multiple) versions of ktlint. So this will not be fixed.

The second issue you didn't mention is also an obvious bug.

This is a direct result of the change I made to fix the original problem. An incorrect line ending is not seen as a format violation itself. Before checking for violation, all code is normalized to lf endings. Once formatting is done, line endings are re-applied according to settings.