Add --newline=[LF|CRLF|native|preserve] option to compile, to override the line separator characters used

[AndydeCleyre]

pip-compile gains an option with two three four valid choices: --newline=[LF|CRLF|native|preserve],
which can be used to override the guessed newline character used in the output file. The default is native preserve, which uses os.linesep tries to be consistent with an existing output file, or input file, or FALLBACK_VALUE (native, or LF? TBD) falls back to LF, in that order.

This aims to address #1448.

Note: poll for fallback value

Contributor checklist

• Provided the tests for the changes.
• Assure PR title is short, clear, and good to be included in the user-oriented changelog

Maintainer checklist

• Assure one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
• Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

[Jackenmen]

IMO \n as that's the line endings that usually get checked into the repository by git due to autocrlf and some other settings. BUT a different potential problem here is that not using native might be considered a breaking change by some. I have no clue whether it would be considered as such by maintainers of this project and I personally wouldn't mind it but it's worth bringing up.

[AndydeCleyre]

I do care about backwards compatibility, but I must note that the current (and "backward" behavior) on Windows is, I think, an inconsistent mix of line endings, depending on annotations and warnings and maybe hashes.

This is because we create some multi line "line" strings for warnings, for example, then join/print these.

[atugushev]

I do care about backwards compatibility, but I must note that the current (and "backward" behavior) on Windows is, I think, an inconsistent mix of line endings, depending on annotations and warnings and maybe hashes.

@AndydeCleyre this looks like a bug. We can fix that inconsistency before the --newline option.

[AndydeCleyre]

What would the backwards compatible behavior be, for windows?

[atugushev]

What would the backwards compatible behavior be, for windows?

I'd guess, If we fix the "inconsistent mix of line endings" then it would be os.linesep. What would you think?

[AndydeCleyre]

I'd guess, If we fix the "inconsistent mix of line endings" then it would be os.linesep. What would you think?

I'd like someone using Windows to confirm. If that is indeed the case, then isn't this PR, in its current state, what is desired anyway?

[AndydeCleyre]

Duplicating my own comment from the relevant issue, for visibility and feedback here:

---

Maybe the move forward is to collpase #1491 into a single new flag, --preserve-newlines (or similar):

• without it, always use LF
• with it, and existing newlines can be detected, use those
• with it, and no existing newlines are detected, use LF

What do you think?

[graingert]

preserve-newlines should be the default. Having no example line separator and forcing pip-compile to guess is so rare (passed from stdin) that the choice barely matters

[AndydeCleyre]

So more like an --LF flag is called for?

[Jackenmen]

preserve-newlines should be the default. Having no example line separator and forcing pip-compile to guess is so rare (passed from stdin) that the choice barely matters

Perhaps it barely matters which line ending is chosen but it does matter for it to be consistent between systems which is not currently the case either.

[AndydeCleyre]

I just threw up draft #1584 based on this PR and one more commit that implements this change and all it entails:

 @click.option(
-    "--newline",
-    type=click.Choice(("LF", "CRLF", "native", "preserve"), case_sensitive=False),
-    default="preserve",
-    help="Override the newline control characters used",
+    "--force-unix-newlines",
+    is_flag=True,
+    default=False,
+    help="Always use LF newlines, rather than auto-detecting from existing files.",
 )

Is that close to what we need?

[AndydeCleyre]

I'd like to direct anyone here to the latest poll back at the source issue, about which behavior should be the default, and what other options should be available, if any. Thanks for any feedback!

[AndydeCleyre]

There aren't very many votes in the poll, but the current winner is still the implementation as it is here in this PR.

[ssbarnea]

Whom from those with review/merge are supporting this feature? I am still -0.5 on it as it would add unrequired complexity to the code as users can control the newlines using git. If I would support something in this area it will enforcing use of LF newlines regardless platform, making it predictable.

@atugushev Are you? Asking because I do not want to see @AndydeCleyre spending more and more time on a task that never gets in.

As I am against, he will need two +1 reviews to get it in.

PS. Sorry, but I am only trying to protect the codebase.

[graingert]

I think it would be neater to use a TextIOWrapper so the existing code can continue to use \n, which should sharply reduce the number of lines changed here

try:
    dst_file = io.TextIOWrapper(self.dst_file, encoding="utf8", newlines=self.linesep):
    for line in self._iter_lines(...):
        dst_file.write(line)
        dst_file.write("\n")
finally:
    dst_file.detatch()

[graingert]

I'm hugely in favor of this feature, and I think using a TextIOWrapper to translate the newlines will keep the changes small enough to justify it

[AndydeCleyre]

@graingert

Thanks!

Closing this in favor of #1652

from +140 −17
to +132 −8