Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unwanted conversion to Windows line endings #493

Closed
kayhayen opened this issue Jan 21, 2017 · 10 comments
Closed

Unwanted conversion to Windows line endings #493

kayhayen opened this issue Jan 21, 2017 · 10 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@kayhayen
Copy link

Hello,

I am using "isort" with a git checkout that doesn't change line endings. When running "isort" on a file, it changes the Unix new lines into Windows new lines, generating not only a huge diff, but also an unwanted change. Effectively this means I can't use isort on a mixed platform project as e.g. a commit hook or so.

Can you please keep the new line convention used and / or allow to disable usage Windows new files, or allow to specify which kinds of new lines to use.

Thanks,
Kay Hayen

@thebigmunch
Copy link

Was just going to post an issue. This is not just an issue of converting to Windows line endings, though that is the most common case. It's also possible for CRLF line endings to be changed to LF on *nix. Here's why:

isort doesn't set the newline parameter for open. This causes all line endings are translated to \n on reading. Compounding this is that isort uses a hardcoded \n in its modifications. When writing in this way, line endings are translated to os.linesep.

If the newline paramater is set to '' on both ends, no translation will take place. But the hardcoded \n in isort modified lines would have to be dealt with in some way.

@timothycrosley timothycrosley added the enhancement New feature or request label May 9, 2017
@sharkykh
Copy link

If it helps,
To get around a similar issue with a script I made (Python 2.7), I use 'wb' mode on the open function.
Every time I ran my script it forced CRLF line breaks (I'm on Windows). Doing this prevents it.

with open('/path/to/in_file', 'r') as fh:
    lines = fh.readlines()
lines = map(str.strip, lines)
lines = '\n'.join(lines)
with open('/path/to/out_file', 'wb') as fh:  # use 'wb' to avoid line separator conversion
    fh.write(lines)

@Plazmaz
Copy link
Contributor

Plazmaz commented Sep 5, 2017

I would say this is a bug more than an enhancement. I am unable to use this program due to this issue.

@thebigmunch
Copy link

thebigmunch commented Sep 5, 2017

I would say this is a bug more than an enhancement. I am unable to use this program due to this issue.

Quite right. I was going to say as much when it was labeled as enhancement but didn't want to come off as snotty. A tool that sorts imports modifying line endings of entire files is clearly a bug.

I've simply hardcoded the behavior I need for now. But, it's definitely a problem that deserves to be properly labeled at least.

@timothycrosley timothycrosley added the bug Something isn't working label Sep 6, 2017
@timothycrosley
Copy link
Member

Hi Guys! I didn't mean to imply that the issue wasn't important by labeling it an enhancement. In general, I label regressions as bugs and everything else as "enhancements", I'll start changing how I catogorize issues to avoid this confusion.

Thanks!

~Timothy

@thebigmunch
Copy link

I didn't test previous versions, but this behavior was likely a regression anyway as there was a switch to io.open to support universal newlines in 56272f4. codecs.open has no automatic newline conversion, so it wouldn't have displayed the same behavior previous to that.

Plazmaz added a commit to Plazmaz/isort that referenced this issue Oct 12, 2017
Fixes PyCQA#493. Thanks to @thebigmunch for finding the source of the problem!
@thebigmunch
Copy link

Congratulations on making isort a line ending changer on purpose (and a bad one at that) while introducing other failure cases. A heavy-handed workaround already existed, adding one to the software wasn't necessary. I, for one, will stop using this tool for deciding to half-ass its job.

@kayhayen
Copy link
Author

Thanks, looking forward to seeing it in a release.

@timothycrosley
Copy link
Member

@thebigmunch you have obviously put a lot of thought and consideration into this issue, the change that was merged was indeed not a 100% fix, but it has not been released into PYPI, and it forces some fix to be made before a PYPI release (which is an important promise in my mind to the users who are running into this issue). I will try to make time to improve the state of the change before release, but I feel like it's very counterproductive and hypocritical of you to complain about "half-assing" without taking the time to put together a pull request yourself. No one gets paid to work on this, and I personally, and probably most users don't and wouldn't work in mixed mode development environments, instead solely using Linux. You are as much responsible and in control of getting what you want as anyone else.

@Ovyerus
Copy link

Ovyerus commented Feb 6, 2019

It seems this issue is cropping up again (at least in my recent experiences), converting LF to CRLF without me wanting it to (on Windows). I'm also having it seem to duplicate newlines as well, but I'm unsure if this is related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants