Parser: Use an HTML like attribute syntax for comment attributes

[youknowriad]

I think HTML like attribute syntax is more solid than our current attributes syntax.

[aduth]

Can you elaborate on what specific benefits you see to these changes

If there were one pain point I'd like to see solved, it would be being able to make some distinctions on types, specifically numbers.

[nylen]

What does this new url attribute add? Let's have a test for spaces and some special characters like '& instead.

[youknowriad]

Yes, handling spaces and special chars like these is one of the reasons I made this switch

[dmsnell]

I'd recommend looking back at wp-post-grammar where I've replaced the colon-separated attributes with HTML like attributes for an implementation that can handle different quote styles

https://github.com/Automattic/wp-post-grammar/blob/dac539b126df6012de6a8a7c3b70b9ad8988f514/src/post.pegjs#L127

[nylen]

Shouldn't matter either way, but is this change intentional?

Separately, it might be nice to run the CI build against both parsers.

[youknowriad]

Not intentional, I was switching to try both.

[nylen]

Here are a couple of benefits of moving to HTML-like attribute syntax:

• More consistency with the rest of the markup (HTML-like attributes look like HTML)
• It's possible to include spaces inside attributes now

I think this is reasonable as an incremental improvement that will allow us to implement the embed block with cleaner serialized markup.

[youknowriad]

@aduth

Can you elaborate on what specific benefits you see to these changes

Yes, storing spaces, tabs ... inside args and remove the ambiguity between the ':' of the name, value separation and any ':' in the attribute value (urls).

If there were one pain point I'd like to see solved, it would be being able to make some distinctions on types, specifically numbers.

Yes, this is another pain point to be solved but I think your idea in #609 is a great solution for this.

[youknowriad]

Updated, I've used the proposed grammar by @dmsnell. It's way better.
And I'm unit testing both parsing methods.

[nylen]

Latest changes look good, nice work on testing both parsers at the same time.

Another potential benefit of HTML-like attribute syntax is that we are intentionally limiting the data types of attributes (currently, to either strings or pseudo-boolean values). If we do allow storing arbitrary data structures it seems like this could quickly get out of hand.

[aduth]

It appears we overlooked updating the serializer here:

https://github.com/WordPress/gutenberg/blob/1cb599d/blocks/api/serializer.js#L62

You can see some breakage when switching to and from the Text mode (Edit: Appears you have to make a change in Text mode before switching back to Visual).

[dmsnell]

What do y'all feel about not pulling in the boolean flag syntax for HTML attributes.

<item checked />

my reason for not wanting to do so is twofold:

• if we do that we're blessing boolean flags and I'm not sure that's a good thing to do here when we could demonstrate more descriptive attributes like type: 'mosaic' instead of isMosaic
• we don't have any demo content with this syntax and I'm not sure it maps well to blocks. on an HTML level we have a small subset of elements (like the select/option groups) but at a block level? I'd like to know what would demand it

cc: @mtias

[aduth]

I'd actually overlooked that this had been introduced here. I too feel it's a bit odd to support now two types (strings, booleans) but not others (numbers). And that there'd not been a use-case driving its inclusion.