Format inline HTML tags #1407

madman-bob · 2018-06-01T11:18:28Z

Replaces concept of unformatted tags with inline tags, which are not wrapped. Previous unformatted functionality moved to content_unformatted tags. This allows us to format the attributes of inline tags correctly.

By comparison with textarea, note that <textarea><textarea></textarea></textarea> is not balanced, as there is actually only one opening tag. The second is raw text inside the text area.

bitwiseman · 2018-06-04T19:26:41Z

@madman-bob
This is very exciting. :)

One problem I see so far is this is going to be a breaking change as you've written it. That would mean having to update the beautifier to v2 and I only want to do that as part of larger changes.

Would you be willing to modify it to make it so it is non-breaking? Specifically, if someone uses unformatted it should continue to work.

madman-bob · 2018-06-05T10:47:34Z

Understandable. Would it be acceptable to alias unformatted to unformatted_content? It would still change the behaviour of unformatted, but only by formatting the attributes, when it didn't before. I can't imagine cases where you wouldn't want to format the attributes.

On an unrelated note, do you have a preferred letter for the command line name of inline? All the obvious ones seem to be taken.

bitwiseman

@madman-bob
I don't have a good one-letter option. That's it secondary concern - this can go public without that.

As to aliasing unformatted to unformatted_content:
Tough call. Most users would want unformatted to alias to inline. But others would want the behavior to not change at all.

I think the right choice is to have the behavior of the unformatted option not change at all, but have it be empty be default (with current values moved to inline).

bitwiseman · 2018-06-05T20:40:49Z

test/data/html/tests.js

@@ -1010,23 +1010,23 @@ exports.test_data = {
            ]
        }, {
            fragment: true,
-            input: '<div><p>Beautify me</p></div><p><p>But not me</p></p>',
+            input: '<div><p>Beautify me</p></div><p><div>But not me</div></p>',


Please keep the original test inputs. You can (and should) create new tests with new values, but wherever possible the tests with the original inputs should also be kept. Then if the original inputs produce changed output it is clear what has changed.

In this case, the new tests are definitely interesting. Please add them.

This test wasn't testing what it was intended to test.

Here, p is content_unformatted, so the final </p> tag is unmatched, in the same way that in

<textarea><textarea></textarea></textarea>

we have the opening tag <textarea>, the literal text "<textarea>", followed by the closing tag </textarea> to match the first tag, and then the unmatched tag </textarea>.

bitwiseman · 2018-06-05T21:07:36Z

test/data/html/node.mustache

-        // bth('<div><span>content</span></div>');
+        bth('<div><span>content</span></div>');
+
+        test_fragment('<p>Should remove <q><span \n\nclass="some-class">attribute</span></q> newlines</p>',


Instead of just this one test, add a whole new section showing the accurate behavior of inlines.
You also need to show them behaving correctly in line-wrap situations.

bitwiseman · 2018-06-05T21:17:48Z

js/lib/beautify-html.js

@@ -237,7 +237,7 @@ function Beautifier(html_source, options, js_beautify, css_beautify) {
    indent_character = (options.indent_char === undefined) ? ' ' : options.indent_char;
    brace_style = (options.brace_style === undefined) ? 'collapse' : options.brace_style;
    wrap_line_length = parseInt(options.wrap_line_length, 10) === 0 ? 32786 : parseInt(options.wrap_line_length || 250, 10);
-    unformatted = options.unformatted || [
+    inline_tags = options.inline_tags || [


The public option is called inline, not inline_tags.

bitwiseman · 2018-06-05T21:34:02Z

@madman-bob
If keeping the existing unformatted behavior is a serious blocker, I'm open to having it map to inline or content-unformatted, which ever produces the least changes (or at least the most sensible changes) from the existing output.

People use unformatted to work with xml documents and add templating elements, it may be hard to get the behavior to not break those scenarios.

In all cases add tests (or even examples) is going to be vital.

madman-bob · 2018-06-08T13:18:49Z

Have now added unformatted tags back in, and some more tests.

madman-bob · 2018-06-19T12:23:13Z

Are there any further changes required?

bitwiseman · 2018-06-20T18:33:31Z

js/src/html/beautifier.js

@@ -229,6 +239,17 @@ function Beautifier(html_source, options, js_beautify, css_beautify) {
            }
        };

+        this.get_last_tag = function() {
+            var last_tag;
+            for (var i = this.output.length - 1; i >= 0; i --) {


Walking back one character at a time is going to have really bad performance characteristics.
At minimum, we should store the last_tag info when we find a tag and then replace it each time a new tag is found.

This is walking back one line at a time, and, while I wrote it defensively, I cannot think of any situation where it would loop more than twice.

bitwiseman · 2018-06-20T18:53:22Z

js/src/html/beautifier.js

@@ -182,6 +184,14 @@ function Beautifier(html_source, options, js_beautify, css_beautify) {
                    }
                }
                return false;
+            },
+            get_tag_name: function(full_tag) {


This information already found during the process of processing the tag: https://github.com/madman-bob/js-beautify/blob/def3cbda7911a343ff603b81a03d63670db456a4/js/src/html/beautifier.js#L534

It would be better to store or pass that information back form there.

The linked line only gets the tag name, while this function also distinguishes whether it's an opening/closing tag. Ideally, I'd refactor the other locations to use this function, but that would introduce minor behavioural changes.

bitwiseman

@madman-bob
I need to run some local tests.
I want @garretwilson to test this before it is released.
And a couple implementation points.

bitwiseman · 2018-06-30T04:08:30Z

@madman-bob
Thanks!

Clarify content_unformatted tests

468f79a

By comparison with textarea, note that <textarea><textarea></textarea></textarea> is not balanced, as there is actually only one opening tag. The second is raw text inside the text area.

madman-bob mentioned this pull request Jun 4, 2018

not correctly joining lines for HTML #1033

Closed

bitwiseman requested changes Jun 5, 2018

View reviewed changes

madman-bob added 3 commits June 8, 2018 14:16

Add concept of inline HTML tags

0e9ad61

Uncomment previously broken test

683a3ac

Add inline element tests

49ed021

Add unformatted HTML element tests

def3cbd

bitwiseman reviewed Jun 20, 2018

View reviewed changes

bitwiseman requested changes Jun 20, 2018

View reviewed changes

bitwiseman added 5 commits June 27, 2018 13:42

Merge branch 'master' into format-inline-tags

bdc6b9c

Merge branch 'master' into format-inline-tags

aeba002

Cleaned up inline to not need more regex

8b85d77

Removing complex is_unformatted check

b6b8472

Minor additional tweaks

bdb9837

bitwiseman approved these changes Jun 30, 2018

View reviewed changes

bitwiseman merged commit 108e4e5 into beautifier:master Jun 30, 2018

garretwilson mentioned this pull request Jul 9, 2018

Remove unformatted defaults as per js-beautify bug fix. Glavin001/atom-beautify#2180

Closed

chibicode mentioned this pull request Aug 11, 2018

HTML formatter adding white space - now fixed on jsbeautify microsoft/vscode#56205

Closed

garretwilson mentioned this pull request Aug 27, 2018

Remove unformatted defaults as per bug fix released in js-beautify 1.8.0. Glavin001/atom-beautify#2210

Closed

ang-zeyu mentioned this pull request Jul 19, 2020

Fix missing line break at end tag #1819

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Format inline HTML tags #1407

Format inline HTML tags #1407

madman-bob commented Jun 1, 2018

bitwiseman commented Jun 4, 2018

madman-bob commented Jun 5, 2018

bitwiseman left a comment

bitwiseman Jun 5, 2018

madman-bob Jun 6, 2018

bitwiseman Jun 5, 2018

bitwiseman Jun 5, 2018

bitwiseman commented Jun 5, 2018 •

edited

Loading

madman-bob commented Jun 8, 2018

madman-bob commented Jun 19, 2018

bitwiseman Jun 20, 2018

madman-bob Jun 25, 2018

bitwiseman Jun 20, 2018

madman-bob Jun 25, 2018

bitwiseman left a comment

bitwiseman commented Jun 30, 2018

Format inline HTML tags #1407

Format inline HTML tags #1407

Conversation

madman-bob commented Jun 1, 2018

bitwiseman commented Jun 4, 2018

madman-bob commented Jun 5, 2018

bitwiseman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bitwiseman commented Jun 5, 2018 • edited Loading

madman-bob commented Jun 8, 2018

madman-bob commented Jun 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bitwiseman left a comment

Choose a reason for hiding this comment

bitwiseman commented Jun 30, 2018

bitwiseman commented Jun 5, 2018 •

edited

Loading