Remove unformatted defaults as per bug fix released in js-beautify 1.8.0.

[garretwilson]

Good news! In beautifier/js-beautify#1033 we finally succeeded in getting a long-standing js-beatify bug fixed. Specifically js-beautify was mis-using the unformatted setting to indicate whether elements were inline or not. Without this set, inline elements such as <mark> would be broken into multiple lines. But with it set, js-beautify would not format these elements at all (e.g. it would not remove line breaks between attributes).

This fix how now available in js-beautify 1.8.0: https://github.com/beautify-web/js-beautify/releases/tag/v1.8.0

The solution was to make a new inline setting that the formatter really considers to be inline—that is, they are formatted, just not broken. All the inline elements formerly in unformatted were moved to inline. The unformatted was left, just in case someone still wants to add an element that really is not formatted at all, but it is set to empty. See beautifier/js-beautify#1407 .

Unfortunately atom-beautify sets its own default values for unformatted, so they will need to be removed or they will override these changes (essentially un-fixing the fix for beautifier/js-beautify#1033 ).

I think the real solution here is to remove the atom-beautify explicit values altogether, as I recommended in #1008 (comment) . js-beautify has its own defaults. They have been changed multiple times, and if atom-beautify duplicates those defaults explicitly, atom-beautify will have to be playing catch-up with manual modifications each time. I think atom-beautify should allow anything to be overridden, but has no need to specify explicitly what atom-beautify is providing as defaults anyway.

In any case, atom-beautify needs to remove the explicit unformatted values set in #1008 whenever atom-beautify updates to js-beautify v1.8.0.

Thanks!

[ghost]

Please follow the issue template provided. More specifically, update the original comment for this issue by adding a link to the required debug.md gist which includes debugging information that answers our most commonly asked questions. Thank you.

[garretwilson]

Sufficient information is provided in the description. There is no need for a debug.md gist.

[Glavin001]

Sorry about that, the issue-complete bot is a little over excited sometimes 😝 . cc @szeck87 .

---

Unfortunately, removing default values for options has already been investigated and determined to not be feasible within Atom: #894 (comment)
I had to hunt for that comment, way back in Jan 3, 2017 😮. Atom's Settings View may have changed since, however, I do not have time to reinvestigate this.

The applicable code for the default options can be found at

atom-beautify/src/options.json

Lines 365 to 433 in bb9428a

	"default": [
	"a",
	"abbr",
	"area",
	"audio",
	"b",
	"bdi",
	"bdo",
	"br",
	"button",
	"canvas",
	"cite",
	"code",
	"data",
	"datalist",
	"del",
	"dfn",
	"em",
	"embed",
	"i",
	"iframe",
	"img",
	"input",
	"ins",
	"kbd",
	"keygen",
	"label",
	"map",
	"mark",
	"math",
	"meter",
	"noscript",
	"object",
	"output",
	"progress",
	"q",
	"ruby",
	"s",
	"samp",
	"select",
	"small",
	"span",
	"strong",
	"sub",
	"sup",
	"svg",
	"template",
	"textarea",
	"time",
	"u",
	"var",
	"video",
	"wbr",
	"text",
	"acronym",
	"address",
	"big",
	"dt",
	"ins",
	"small",
	"strike",
	"tt",
	"pre",
	"h1",
	"h2",
	"h3",
	"h4",
	"h5",
	"h6"

However, I am not necessarily in favour of making a breaking change -- updating the defaults -- as it is recommended to users of Atom-Beautify to explicitly override with their own option values and note again the default options are only there to appease Atom's Settings View schema validation. Atom-Beautify is not trying to be the best and single source of truth for default options. Atom-Beautify is trying to allow users the freedom to change these options in a similar way with a single package.

The solution was to make a new inline setting that the formatter really considers to be inline—that is, they are formatted, just not broken. All the inline elements formerly in unformatted were moved to inline. The unformatted was left, just in case someone still wants to add an element that really is not formatted at all, but it is set to empty. See beautifier/js-beautify#1407 .

I only have a few moments right now -- however, I wanted to reply ASAP -- so I have yet to review js-beautify's issue fully.

At first glance, it sounds like a new inline option should be added to Atom-Beautify? If this is the case, I would be happy to review a small Pull Request submitted from an open-source community member, such as yourself @garretwilson . I do want to support the latest and greatest options, such as inline for js-beautify, and welcome the community to submit Pull Requests for @szeck87 and I to review.

The default values are not something I want to maintain and only a necessary evil, as mentioned above and demonstrated in #894 (comment) . Given we cannot simply remove the default values and also doing so would be an unexpected breaking change for some, I would not approve a Pull Request implementing such a change without further consideration.

I'll try to review js-beautify later. Please comment here if you have any questions or suggestions. Thank you!

[bitwiseman]

@garretwilson @Glavin001
I thought I'd take a swing at fixing this, but I don't think I'm doing it right.

[garretwilson]

At first glance, it sounds like a new inline option should be added to Atom-Beautify?

Yes, but (and this is important!) you much change the defaults of unformatted to [].

Let me make it simple: unformatted should always have been empty by default. <a>, for example, should not be "unformatted". We want to format an <a> tag. I want to format <a>. You want to format <a>. We all want to format <a>. If there are any people (rare, I'm sure) who do not want to format <a>, they can manually set it in their own unformatted configuration, like you said.

Then why did we put <a> in unformatted before? Because there was an atrocious bug that would format <a> (and everything else) as if it were a block element!! It would put newlines before and after <a>, and other (perhaps all other) inline elements. So we put all the inline elements in unformatted just to work around this bug.

This bug has (finally!!!!!!) been fixed. The inline elements are now in inline. So you need to do two simple things:

• Add an inline setting that defaults to all the inline elements (i.e. the values unformatted used to have).
• Set the default to unformatted to [].

There is no reason to keep unformatted to anything but [] now. If you do not change the default of formatted to [], then that means everybody who wants to format <a> (which is probably 99.997 percent of your users) will be forced to put "unformatted": "" in their configuration file or the file won't format correctly.

Even worse, if users don't know to put "unformatted": "" in their configuration file, they will think their files are formatting correctly until that rare instance comes along and their <a> has newlines in the middle of it, and atom-beautify won't format it, and they won't know why it's broken.

Please, please set the default of unformatted to [] and not force all your users to set it manually.

[garretwilson]

Let me give an example, assuming that line-wrapping is turned off:

<p>This is a <a


    href="http://example.com/">link</a> to a site.</p>

Does that need formatting? Yes, of course. It should look like this:

<p>This is a <a href="http://example.com/">link</a> to a site.</p>

Can you find one single person who says that <a> should not be formatted? I seriously doubt you can. (And if that one person exists, they can set "unformatted": ["a"].)

The reason we had <a> in the unformatted list before, is that there was a horrible bug that would format things something like this:

<p>This is a
  <a href="http://example.com/">link</a>
  to a site.
</p>

And so to work around the bug we chose the lesser of two evils and decided to tell js-beautify not to format <a> at all! (And we did it with <cite>, and <abbr>, and <code>, and <mark>, etc.) But that's not really what we wanted. That's not what anyone wants!

That bug has been fixed. We can now say, "Format <a> by default, but don't add newlines before and after it." If you only add the new inline setting, then you're only doing the second part: "Don't add newlines before or after <a>." But you'll still be leaving in a setting that says, "Don't format <a> at all!!" What's the point in that?

[garretwilson]

In order to show that I don't merely want to complain, but that I do want to contribute, I've posted a USD$50 bounty for this ticket. I think @bitwiseman has already submitted a pull request; that's fine—he certainly deserves it for going out of his way to update another library that depends on his.

Here is the bounty for whoever completes this task:

https://www.bountysource.com/issues/62674449-remove-unformatted-defaults-as-per-bug-fix-released-in-js-beautify-1-8-0

I'm so anxious to get this fixed and I'll be overjoyed when it's in. Thank you in advance.

[bitwiseman]

@garretwilson
As usual you make a thorough case for your desired change.

FYI, you (and any other user) can get the behavior you want by setting unformatted to empty right now. Since atom-beautify doesn't currently set inline, it will use the default from js-beautify. That should reduce the personal urgency of this change. :).

[garretwilson]

As if I didn't have enough to complain about: I just now uninstalled atom-beautify and tried to install it again, to pull down the latest js-beautify. I can't even install atom-beautify back in Atom!! I get the following:

npm WARN deprecated [email protected]: please upgrade to graceful-fs 4 for compatibility with current and future versions of Node.js
npm WARN deprecated [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated @types/[email protected]: This is a stub types definition for commander (https://github.com/tj/commander.js). commander provides its own type definitions, so you don't need @types/commander installed!
npm WARN deprecated [email protected]: Deprecated in favour of eslint-config-wikimedia. -- https://phabricator.wikimedia.org/T118941
npm WARN deprecated [email protected]: Package no longer supported. Contact [email protected] for more info.
npm ERR! Windows_NT 10.0.17134
npm ERR! argv "C:\Users\user\AppData\Local\atom\app-1.30.0\resources\app\apm\bin\node.exe" "C:\Users\user\AppData\Local\atom\app-1.30.0\resources\app\apm\node_modules\npm\bin\npm-cli.js" "--globalconfig" "C:\Users\user\.atom\.apm\.apmrc" "--userconfig" "C:\Users\user\.atom\.apmrc" "install" "C:\Users\user\AppData\Local\Temp\…\package.tgz" "--runtime=electron" "--target=2.0.5" "--arch=x64" "--global-style"
npm ERR! node v6.9.5
npm ERR! npm v3.10.10
npm ERR! code EREADFILE

npm ERR! Error extracting C:\Users\user.atom.apm\marko\4.13.3\package.tgz archive: ENOENT: no such file or directory, open 'C:\Users\user.atom.apm\marko\4.13.3\package.tgz'
npm ERR!
npm ERR! If you need help, you may report this error at:
npm ERR! https://github.com/npm/npm/issues

npm ERR! Please include the following file with any support request:
npm ERR! C:\Users\user\AppData\Local\Temp\…\npm-debug.log

(grumble grumple) Now I'm in a pickle. Maybe it's because today I upgraded Node to v10.9.0. Whatever the case, Node won't let me downgrade.

Any ideas? Now I don't even have any formatting plugin in Atom.

[garretwilson]

Nope, I uninstalled Node and reinstalled v9.5.0 I get the same error trying to install atom-beautify. What's going on?

[garretwilson]

Apparently it's a known problem: https://discuss.atom.io/t/installing-atom-beautify-0-33-0-failed/58383

I don't want to hijack this thread, but frankly I'm just too tired today to file yet another atom-beautify bug.

[garretwilson]

Installing atom-beautify is completely broken. See also https://stackoverflow.com/q/52124888/421049 . Installing via apm doesn't work.

[garretwilson]

So it's already reported in #2213 , and the stupid bot keeps closing that ticket. It seems many of us are in a pickle.

[garretwilson]

Following this tip I temporarily created a symbolic link to marko 4.13.4 to make it think it was marko 4.13.3, and at least got atom-beautify installed again. But that's a pretty big issue. Few people will be able to sift through all the threads and figure out this whole marko versioning problem and how to get around it (albeit not as few as the people who really don't want to format <a> and want unformatted set to all the inline elements 😛).

[stevenzeck]

@garretwilson I published a new version of Atom Beautify yesterday that resolved the installation issue. But for the purposes of developing atom beautify, you'll need to clone it locally and symlink it to Atom itself anyways.