Erroneously changes CRLF to LF on Windows.

[garretwilson]

atom-beautify (which uses js-beautify) at first seemed nice, but then I realized it changed all my CRLF line endings on Windows to LF. 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.

This is a huge blocker issue. I can't have something mucking around with my line endings. This messes up my workflow, especially with Git.

[bitwiseman]

There is a manual option for this. What you're requesting is automatic detection, right?

It appears that there is extensive discussion occuring under
Glavin001/atom-beautify#707, also referencing
prettydiff/prettydiff#186.

[garretwilson]

The problem is simple: My file on Atom starts out with CRLF line endings. After atom-beautify, the files have LF endings. This is unacceptable. That's it---no more, no less.

But I don't know where the problem lies. Is js-beautify failing to even consider the line endings already existing? Or does the problem lie with atom-beautify not telling js-beautify which line-ending style to use?

I find it curious that Komodo's Beautify js plugin does not suffer from this problem.

[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]

So I'm back to the same question:

Is js-beautify failing to even consider the line endings already existing? Or does the problem lie with atom-beautify not telling js-beautify which line-ending style to use?

Are you implying that since the Komodo add-on doesn't tell js-beautify anything special, that this is a problem with js-beautify itself? Or a bug in Atom?

[bitwiseman]

I'm saying that if we follow Komodo's example, it is not the beautifier's responsibility to choose the newline for a file. If there is a bug here it is in the atom-beautifier, which should tell js-beautifier what the newline should be.

That said, I'm going to keep this as an enhancement request.
The behavior would be if opts.eol is set to auto, the beautifier would try to detect the newline character based on the first newline found in the file.

[garretwilson]

The behavior would be if opts.eol is set to auto, the beautifier would try to detect the newline character based on the first newline found in the file.

Yes, that would be awesome. Have you scheduled this for a release yet?

[garretwilson]

@bitwiseman , please clarify the status of this. You indicated that pull request #846 was supposed to add "auto" as the default value of the eol option. You also added the v1.6.0 milestone to this bug.

Your main page https://github.com/beautify-web/js-beautify indicates that you are at version 1.6.2, yet the documentation on that page does not indicate an auto option for eol.

Moreover https://github.com/Glavin001/atom-beautify doesn't seem to support the "auto" value for eol, even though atom-beautify has been updated to js-beautify 1.6.2.

Can you clarify when eol = "auto" support is really being released? Thank you very much.

[bitwiseman]

@garretwilson -
First off, and I mean this in all good humor, RTFC. Only an inch or so above your comment is the link to the commit that I "indicated" fixed this issue. I'll add here as well: bitwiseman@796f080 . If you click on the link, it will take you a wonderful page showing the code that claims to fix the issue. Take a look at it.

Second, once you've looked at it, decide whether it is sufficient. I just looked at it again and it seems to be in okay but the documentation changes aren't quite good enough. This (bitwiseman@796f080#diff-319aee52866e5899a11a7f49987984b4R273) shows the change in default behavior, but doesn't mention 'auto' as a new valid value. Also the README.md didn't get updated.

Thirdly, this project is not atom-beautify, and if you look at this issue you'll see it linked to the still open issue on the atom-beautify project. Glavin001/atom-beautify#707 . It doesn't mean the fix to this project isn't "really" released (it is), it means atom-beautify hasn't finished the changes needed to support it on their side. Maybe once the README is updated, they'll get to. Or maybe you want to mention it to them.

Finally, and again I mean this in the most friendly way possible, the tone of your written communication needs improvement. I'm sure you don't mean to, but your comment (and I will say, many of your other comments) comes across as a challenge to my competency and/or honesty. If it were me, I might have written something like this instead.

@bitwiseman, thanks for putting in this fix.

It looks like you've made auto the default behavior, which is exactly what I was asking for.

I think you forgot to update README.md. Oh, and I don't see auto mentioned as a option for eol. I've put in a pull request (link) to address these, please take a look.

I also don't see this option in atom-beautify. It looks like Glavin001/atom-beautify#707 isn't fixed yet, even though they've upgraded to 1.6.2. Do you know if there's anything that needs to be done in that project to make this fix available?

Thanks again!

In the short term, your tone is fine - challenging questions will tend to get a faster response because people feel the need to defend when attacked. However, in the long run, people will tend to respond more readily and productively to people that are friendly, people they feel are on the same team. I know that's true for me personally.

I hope you find this feedback constructive. I've found your input on this project useful, and I hope you continue to report issues and request new features.

[garretwilson]

Seriously, @bitwiseman , there was no malice there. Things confused me. I listed the things that confused me so that you would see why I was being confused, and I asked for clarification. I frankly had no idea of what was going in, because everything seemed to be pointing to different answers, as I indicated.

So let me see if I understand that you're saying: you're saying that you think to checked in a fix, but maybe it's not working? (I'll re-read this again in the morning; my mind is frazzled right now, as I've been giving programming lessons all afternoon and evening.)

[garretwilson]

P.S. I will try to add lots of 😃s and 😛s so my posts will sound much nicer. 😄

I'm writing an entire set of materials for a programming class using XHTML5. It is exhausting when everything breaks just trying to edit XHTML5. Bluegriffon corrupts files. Komodo is using a horribly outdated version of Tidy that doesn't recognize modern elements. Your plugin changes my line endings. The Atom tidy linter is broken. The other formatters are broken. Nobody understands the XML declaration so I had to take it out. I could go on and on and on. I would have thought that formatting HTML would have been something they would have solved back in 1995.

But seriously, all I want is something to work. I promise I want to help, and I promise I'll try to be diplomatic. Just let me know what the status is and what I can do to help this go forward.

[bitwiseman]

Ah, yes, that sense of frustration was why I started contributing to this project. So, I feel your pain. You'd think this would have been "solved" long ago, but it hasn't.

So let me see if I understand that you're saying: you're saying that you think to checked in a fix, but maybe it's not working? (I'll re-read this again in the morning; my mind is frazzled right now, as I've been giving programming lessons all afternoon and evening.)

Nope. I'm saying I did checkin a fix to this issue. It is working (the commit I linked to shows tests verifying that it is working). It also shows the documentation was not fully updated to match. Also, atom-beautify is a separate project, and they need to make changes on their end to make this fix available on their end.

[garretwilson]

[atom-beautify] need to make changes on their end to make this fix available on their end.

What is that exactly? I looked at your code and I thought that you had made the "eol" setting default to "auto". And that's what I would expect the default to be. So shouldn't it just "work" as long as no one overrides the "eol" setting? (I'm just trying to figure out what part I don't understand.)

[bitwiseman]

I believe the eol value is actually set to a value by atom-beautify, which would override the auto.

If you believe this isn't working on the js-beautify side, you can test it by saving your file and then running the js-beautify command on the file.

[garretwilson]

If you believe this isn't working on the js-beautify side, you can test it by saving your file and then running the js-beautify command on the file.

Yes, and as it turns out, it indeed seems that this is not working in the js-beautify side.

I installed [email protected] via npm. I use the following .jsbeautifyrc config file:

{
    "eol": "\r\n",
    "indent_with_tabs": true,
    "preserve_newlines": true,
    "max_preserve_newlines": 2,
    "end_with_newline": true,
    "wrap_line_length": 0
}

Here's my input file (using CRLF):

<!DOCTYPE html>
<html>
  <head>
    <meta  charset="utf-8" />
    <title>Foo</title>
  </head>
  <body>
    <h1>Foo</h1>
  </body>
</html>

I run:

js-beautify --type html -f in.xhtml -o out.xhtml -c .jsbeautifyrc

The output file has CRLF, just as specified in .jsbeautifyrc. Good.

Next I change my .jsbeautifyrc to the following:

{
    "eol": "\n",
    "indent_with_tabs": true,
    "preserve_newlines": true,
    "max_preserve_newlines": 2,
    "end_with_newline": true,
    "wrap_line_length": 0
}

I get an output with LF line endings. Good again. Just what I asked for. But remember, this is existing functionality. We want to try the EOL auto-detect feature. So I remove the eol designation altogether from the .jsbeautifyrc file:

{
    "indent_with_tabs": true,
    "preserve_newlines": true,
    "max_preserve_newlines": 2,
    "end_with_newline": true,
    "wrap_line_length": 0
}

The output uses LF for line endings. 😦 (Remember that the input file had CRLF as line endings.) So EOL auto-detect doesn't seem to be working.

For completeness I try the auto option:

{
    "eol": "auto",
    "indent_with_tabs": true,
    "preserve_newlines": true,
    "max_preserve_newlines": 2,
    "end_with_newline": true,
    "wrap_line_length": 0
}

Here's what I get as output:

<!DOCTYPE html>auto<html>autoauto<head>auto <meta charset="utf-8" />auto    <title>Foo</title>auto</head>autoauto<body>auto <h1>Foo</h1>auto</body>autoauto</html>auto

Yes, this is exactly the behavior I reported for atom-beautify because that's where you directed me.

Thirdly, this project is not atom-beautify, and if you look at this issue you'll see it linked to the still open issue on the atom-beautify project. ... It doesn't mean the fix to this project isn't "really" released (it is), it means atom-beautify hasn't finished the changes needed to support it on their side.

See, @bitwiseman, that's why I asked you about the status. Maybe my tone was a bit frustrated, but surely you can see why sometimes I get the feeling that js-beautify and atom-beautify are playing hot-potato.

I think it's clear now that, unless I'm doing something incredibly wrong (maybe I didn't RT correct FC), the error is firmly on js-beautify's side.

[bitwiseman]

And finally, since you've given a command-line repro of the issue, I can instantly see where the problem is. You're doing HTML beautification, which doesn't have this feature implemented yet.

[bitwiseman]

@garretwilson

See, @bitwiseman, that's why I asked you about the status. Maybe my tone was a bit frustrated, but surely you can see why sometimes I get the feeling that js-beautify and atom-beautify are playing hot-potato.

Dude, we have two independent components. The source of a problem is not always clear. And as I said on the other issue, if you'd done what I suggested originally - "If you believe this isn't working on the js-beautify side, you can test it by saving your file and then running the js-beautify command on the file." - we would have found the root cause more quickly.

If it seems we're playing hot-potato, it's because you're behaving like one. 😄 The more you can calm down, assume best intentions, and get us the information we ask for, the faster we'll be able to help you.

[garretwilson]

And finally, since you've given a command-line repro of the issue, I can instantly see where the problem is. You're doing HTML beautification, which doesn't have this feature implemented yet.

Ah, did I not mention I was using HTML? Silly me. I guess that after filing HTML bug #828, HTML bug #830, HTML bug #841, and HTML bug #882, I forgot to mention here in #829 that I was using HTML. That was my oversight; I'll try to be more careful about that.

But let me be honest that filing so many bugs just trying to get my HTML formatted has taken a lot of my time, which may explain why I wasn't so eager to do yet more testing for something that was obviously broken (and still is broken, as you note). I hate to have troubled you so much, and I also hate that I've wasted so much of my own time and I still don't have correct HTML formatting. I know you have the best intentions, but... at the end of the day I still don't have correct HTML formatting.

But you have to admit that I've put a lot of work in on my part trying to help you solve your problems, from researching the canonical list of inline elements in #840 to laying out an entire formatting conceptual algorithm in #841. I've been complaining, but I feel I've also tried to provide lots of feedback on how to do it right.

But unfortunately it still isn't done right, so I'm thinking maybe I should look into what I can do in a separate effort to get HTML formatting done, seeing that I'm going to have to spend my time on it one way or another. But I do have a lot of experience in (X)HTML standards and processing, so if you have any questions in the future I encourage you to drop me a note and I'll do my best to provide helpful feedback.

All the best.