Erroneously changes CRLF to LF on Windows

[garretwilson]

atom-beautify at first seemed nice, but then I realized it changed all my CRLF line endings on Windows to LF. How do I turn this off?

This is a bug. I shouldn't have to find a setting to keep my current CRLF line endings. atom-beautify should stay with my current line endings; or at the very, very least detected my system and use appropriate line endings. The current behavior is incorrect as a default.

[garretwilson]

This is a huge blocker issue. I see that someone else opened the same issue (#593) for JavaScript, and it was never resolved.

I can't have something mucking around with my line endings. This messes up my workflow, especially with Git. (I am well-versed in .gitattributes, etc.) This is a bug. It needs fixed. Until then, I'm dumping atom-beautify. Not to be mean, but because it's messing up my files.

[garretwilson]

I don't know which side this bug is on, so I have filed it on js-beautify.

[prettydiff]

The default beautifier for JavaScript is JS Beautify. If they don't have an issue open on this then open an issue there. We do not control which options they choose to accept or when those options are completed.

Pretty Diff added this feature with prettydiff/prettydiff#186 and the addition of this option to Atom Beautify is being tracked at: #524

[garretwilson]

What is odd is that the Komodo Beautify js add-on doesn't change the line endings. I wonder how Komodo can use Beautify without ruining my line endings, yet atom-beautify cannot?

[prettydiff]

The difference is that the line termination sequence is designated by the IDE and not the beautifier. The beautifier must just be willing to accept an optional definition for the line termination sequence and apply it appropriately. Many IDEs are like this in that they allow the user to choose an accept line termination sequences from a list for any given file. I have not used Komodo Edit so I don't know if it has the capability. Notepad++ is a Windows only editor that allows the user to change line termination sequence in nearly the same way as Atom.

[garretwilson]

I'm not understanding you. Are you saying that Atom has no clue about what line-ending is being used for a particular file, and therefore cannot pass this option to JS Beautify?

[prettydiff]

Atom Beautify allows a user to choose the line ending per each file. Click on the CR, LF or CRLF that appears on the bottom right corner under your file. You can choose your line ending there. The beautifier has to know that setting in order to apply it to the code at beautification time.

[garretwilson]

Nope. That's the whole point of why I filed this bug.

When you open a file, Atom already detects the line ending (like 90% of the editors out there). It will say "CRLF", "LF", or "mixed". If it says CRLF, Atom Beautify will change it to LF when auto-saving. Erroneously. A bug. Which is why I filed an issue.

If I then take the saved file and manually change "LF" back to "CRLF", and then save the file, Atom Beautify will change it back to "LF". Erroneously. A bug.

Besides, this whole thing about the CR/LF thing that appear at the bottom of the window is a red herring. Atom already has the idea of a default line ending. If I create a new text file, Atom will save if as CRLF on Windows, without my doing anything. That is conventional and expected behavior. So even if the bottom status bar said "mixed", Atom Beautify should know what to do.

So here is the situation:

• If my line ending says "LF", Atom Beautify should leave it at LF. (It does!! This works!)
• If my line ending says "CRLF", Atom Beautify should leave it at CRLF. (It does not. This is a bug.)
• If my line ending says "mixed", Atom Beautify should check the default line ending of Atom Editor and use that line ending when formatting. (It does not. This is also a bug.)

[bitwiseman]

@garretwilson,
The komodo plugin doesn't appear to do any detection. It just has an option that can be set.
babobski/Beautify-js@8ffb551

Maybe that option is set automatically by the IDE somehow, but it doesn't look like the plugin is doing anything.

[garretwilson]

The komodo plugin doesn't appear to do any detection. It just has an option that can be set.

@bitwiseman , you're missing something; the Beautify-js plugin for Komodo auto-detects the line ending. The option you see is only a fallback.

[garretwilson]

@prettydiff and @Glavin001 , please review this issue and the discussion at beautifier/js-beautify#829 . In short, @bitwiseman (the author of js-beautify) believes that, based upon how this is implemented in the beautifier plugin for Komodo, the most appropriate place for dynamically determining the EOL setting should be in the plugin itself, that is, in atom-beautify. After all, the atom-beautify plugin has direct access (as @prettydiff mentioned as well above) to the EOL setting that user has set in the application (i.e. in Atom). The underlying js-beautify library has no direct access or even knowledge of the Atom settings for the file.

@bitwiseman has said that he will investigate adding an option to attempt to auto-detect the file EOL style by looking at the first EOL encountered, but this is only a workaround. The appropriate place for this to be set is in the plugin itself, as it can simply pass in the selected EOL style for the currently loaded document.

Telling me to go create my own js-beautify configuration that specified EOL style would be missing the point. The EOL style needs to be determined, not from a static configuration file, but dynamically based upon the system. Atom already does this; what is lacking is for the atom-beautify plugin to read the selected Atom EOL style and pass it to the underlying js-beautify library. The Komodo plugin https://github.com/babobski/Beautify-js does something similar.

In summary, please add the ability for the atom-beautify plugin to read and/or detect the EOL style of the currently loaded Atom document and pass that to the underlying js-beautify library.

[Glavin001]

In summary, please add the ability for the atom-beautify plugin to read and/or detect the EOL style of the currently loaded Atom document and pass that to the underlying js-beautify library.

Sounds good. We can handle this in Atom Beautify.

The applicable file to change is https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/js-beautify.coffee#L24-L48
Whatever option JS-Beautify requires to be set you can make the change to the options variable before it is passed to JS-Beautify.

Example, add line at https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/js-beautify.coffee#L25

options.eol = atom...getEOL() # <-- whatever the Atom API is that @prettydiff mentioned is available within Atom

I will accept a pull request implement this bug fix. Thank you.

[garretwilson]

I will accept a pull request implement this bug fix.

Yay! Sounds like the first place for me to make a contribution. Looks like I'll have to speed up my study of the Atom plugin architecture. (I'm behind in my reading from filing all these bugs---ack!) Cheers.

[garretwilson]

@Glavin001 , I'm gearing up to check out the code and make the changes. Feel free to add me as a contributor and assign this bug to me if you like.

Regarding the pull request, I haven't contributed to many projects yet on GitHub. Do you prefer that I create a branch and create a pull request for the branch, or do you prefer that I fork the entire repository, create a branch, and then create a pull request from the branch of the forked repository? If the latter, do you prefer that I merge the branch before creating the pull request? Please excuse the newbie question---I'm a long-long-long time Subversion user, and I'm not yet used to the preferred GitHub workflow. Thanks.

[Glavin001]

I think forking the submitting a Pull Request would be ideal. Feel free to create a new branch on your fork, however you could still use master branch in your fork and be able to make a pull request. And yes, please be sure to merge in any of the latest changes to this original repository for your pull request. I'll try my best to review the pull request quickly to mitigate the number of possible merge conflicts over time -- I am busy with work this weekend so there likely will not be anything conflicting pushed upstream. Let me know if you have any more questions. Thanks! 😃

[garretwilson]

Atom Beautify allows a user to choose the line ending per each file. Click on the CR, LF or CRLF that appears on the bottom right corner under your file. You can choose your line ending there. The beautifier has to know that setting in order to apply it to the code at beautification time.

@prettydiff , do you happen to know if this setting is exposed via the API? I see that the editor exposes its encoding, but offhand I don't see anything that would tell the current EOL style. (Of course I can probably guess it by poking around in the text, but I wanted to check first to see if you knew anything offhand.)

[garretwilson]

Regarding checking the current line ending state, I can probably get whatever I need from here: https://github.com/atom/line-ending-selector/blob/master/lib/main.js

[prettydiff]

http://prettydiff.com/documentation.xhtml#crlf

[jerone]

How do I use this setting in Beautify? I'm having the same issue; opening a file with CRLF and after beautifying it's converted to LF.

[garretwilson]

@Glavin001 , I communicated with @bitwiseman and he tells me that js-beautify has an "eol" option that, in the latest version of js-beautify, now defaults to "auto". This "auto" option is supposed to auto-detect the line endings automatically (which would obviate issue #707).

I tested this on atom-beautify, which purports to include the latest js-beautify, but it doesn't work if I remove my .jsbeautifyrc file, which had been setting "eol" to "\r\n".

@bitwiseman seems to think that the reason it isn't working is that atom-beautify is overriding the "eol" value, so if atom-beautify would simply leave the "eol" value alone, the new default value of "auto" would kick in and js-beautify would auto-detect the line endings.

But one of the things that makes it super confusing to me is that I actually changed the "eol" to "auto" in my .jsbeautifyrc, and you know what happened? My "formatted" file become one single line, because atom-beautify replaced all the line endings with the literal value "auto".

My top two guesses (which is why I raised the issue with @bitwiseman):

• Maybe the issue still isn't really fixed in js-beautify.
• Maybe atom-beautify didn't really pull in the latest version of js-beautify.

But I confess I'm completely lost on what's going on. All I know is that... it still doesn't work. 😞

On the positive side, both @Glavin001 and @bitwiseman seem like great people and they are nice to work with, so I hope I can help point them in the right direction to getting this fixed. 😃

[bitwiseman]

But one of the things that makes it super confusing to me is that I actually changed the "eol" to "auto" in my .jsbeautifyrc, and you know what happened? My "formatted" file become one single line, because atom-beautify replaced all the line endings with the literal value "auto".

What exactly did you do to get the result you mentioned? Did you run js-beautify from the command line? Did you check that the version you're running is 1.6.x?

[garretwilson]

What exactly did you do to get the result you mentioned?

I removed my .jsbeautifyrc file and hit Ctrl+Alt+B inside Atom 1.5.3 on Windows 10 Pro 64-bit.

Did you run js-beautify from the command line?

No. I am solely running atom-beautify.

Did you check that the version you're running is 1.6.x?

I am running atom-beautify 0.28.22.

Now, here may be the source of the confusion: I saw on https://github.com/Glavin001/atom-beautify that someone had said, "Bump jsbeautify to 1.6.2." But now I see that this was a commit to https://github.com/Glavin001/atom-beautify/blob/master/CHANGELOG.md , and looking at that file it is not clear to me that atom-beautify has actually released a new version containing the latest js-beautify. Maybe @Glavin001 can clear this up.

[Glavin001]

tl;dr: Uninstall and reinstall Atom Beautify to install the latest js-beautify with the fixes. Then either have a .jsbeautifyrc file forcing the eol option or leave it to being the default (auto) as handled by js-beautify internally.

---

Atom Beautify does not (yet) care about your end of line character(s). Atom Beautify extracts the text/code and the options and hands them off to the beautifier.
It is currently expected the beautifiers (js-beautify, Pretty Diff, etc) detect the EOL and maintain the same EOL in the beautified code.

As noted above, there are options available for both JS-Beautify and Pretty Diff for manually choosing the EOL.
We can easily add support for changing the EOL value that is detected or manually selected by the Atom Editor. See #707 (comment)
I would review and accept a pull request implementing such an enhancement.

so if atom-beautify would simply leave the "eol" value alone, the new default value of "auto" would kick in and js-beautify would auto-detect the line endings.

Looking at https://github.com/Glavin001/atom-beautify/blob/master/src/languages/javascript.coffee I do not see an option for eol, therefore Atom Beautify would not be setting a default.

Furthermore, looking at https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/js-beautify.coffee#L13 I also do not (yet) see any changes to eol before passing the options to JS-Beautify.
So it is not Atom Beautify that is the culprit here. The default for eol option as defined by JS-Beautify internally should apply.

But one of the things that makes it super confusing to me is that I actually changed the "eol" to "auto" in my .jsbeautifyrc, and you know what happened? My "formatted" file become one single line, because atom-beautify replaced all the line endings with the literal value "auto".

My top two guesses (which is why I raised the issue with @bitwiseman):

• Maybe the issue still isn't really fixed in js-beautify.
• Maybe atom-beautify didn't really pull in the latest version of js-beautify.

I would agree with both, in the sense that your copy of js-beautify locally does not have the fix.
Atom Beautify's package.json specifices "js-beautify": "^1.6.2" so if you wish to update js-beautify what I recommend is uninstalling Atom Beautify and reinstalling it, as this will install a fresh copy of the latest dependencies and therefore you will now have the fixed version of js-beautify installed.

[bitwiseman]

Thanks, @Glavin001 !

[garretwilson]

Uninstall and reinstall Atom Beautify to install the latest js-beautify with the fixes. Then either have a .jsbeautifyrc file forcing the eol option or leave it to being the default (auto) as handled by js-beautify internally.

Nope, sorry. I had been through most of that (except for the reinstalling) before filing this report.

I've filed #829 because this belongs in a separate issue.

[Glavin001]

Closing this as a third party bug -- Atom Beautify is working as expected and passing the options directly to js-beautify, as described in #829.
Solution is to update js-beautify or pretty diff by reinstalling Atom Beautify.

For those still having issues, please comment and follow #829 which hopefully we will be resolved soon, once and for all 😃.

[garretwilson]

I will accept a pull request implement this bug fix

@Glavin001 , back in January I promised to add Atom EOL configuration detection to atom-beautify because js-beautify ignored the current EOL settings---which is why I opened this issue #707 in the first place. You indicated that you would accept my pull request once implemented.

Following this there was much confusion when it appeared that the js-beautify project now detected the correct platform EOL. Howerver after much back-and-forth it became apparent that this new js-beautify feature did not apply to HTML beautification, and that such will only be fixed in beautifier/js-beautify#899 . Unfortunately in the meantime you had already closed this issue #707, even though the promised workaround in atom-beautify has not yet been implemented.

Since we have no idea when js-beautify will actually implement EOL detection for HTML files, I have implemented the workaround functionality I had promised originally. I have submitted pull #941, which does the following:

1. Checks the Atom EOL configuration. If it is explicitly set to LF or CRLF, that setting is used.
2. If the Atom configuration is set to OS-based EOL, then I use CRLF for Windows and LF for all other platforms.
3. Otherwise no EOL setting is set.

Note that my approach at no time looks in the buffer to see the EOL character being used in the current document. I believe my approach is the appropriate one, as the whole point of beautification is to modify the document to use some standard formatting (i.e. the current format is not necessarily correct).

I respectfully request that you reopen this issue, accept my pull request, and then close this issue marking it as implemented.

Sincerely,

Garret

P.S. This is my first ever GitHub pull request, and I am also new to CoffeeScript, but I tried to follow best practices in all cases that were evident from my research.

[Glavin001]

Merged #941. Will be published in next release. 👍 Thanks!

[garretwilson]

@Glavin001 , I had originally assumed that the line-ending-selector.defaultLineEnding setting was in the Atom configuration proper, but during my reply to @jerone in pull #941 I realized that it is a Line Ending Selector (core) package setting.

So in the release notes and official documentation you might just mention that js-beautify honors the default line ending setting configured in the Line Ending Selector package settings. (Otherwise it may be confusing as to where users can configure this.) By default it is set to "OS Default", which is what most users will prefer, I would think. Cheers!

[Glavin001]

@garretwilson : Good idea. Feel free to submit another Pull Request changing the documentation however you see fit to assist users with PR #941.

[Glavin001]

Published to v0.29.7