Directives should Store Class Names as an Array

[benrbray]

Initial checklist

• [x] I read the support docs
• [x] I read the contributing guide
• [x] I agree to follow the code of conduct
• [x] I searched issues and couldn’t find anything (or linked relevant results below)

Subject

Markdown:

:::foo[bar]{#baz, .one, .two, .three, key=val}
lorem ipsum dolor sit amet
:::

Mdast:

{
  "type": "root",
  "children": [
    {
      "type": "containerDirective",
      "name": "foo",
      "attributes": {
        "id": "baz,",
        "class": "one, two, three,",
        "key": "val"
      },
      "children": [
        {
          "type": "paragraph",
          "data": {
            "directiveLabel": true
          },
          "children": [
            {
              "type": "text",
              "value": "bar"
            }
          ]
        },
        {
          "type": "paragraph",
          "children": [
            {
              "type": "text",
              "value": "lorem ipsum dolor sit amet"
            }
          ]
        }
      ]
    }
  ]
}

Problem

Currently directive class names like {.one, .two, .three} are returned as a string:

"attributes": {
    "class": "one, two, three,"
},

This is a bit weird, since the class name have been transformed, just not into a useful format. There's also a pesky trailing comma to remember when parsing the string manually. I would expect any of the following instead:

• (bad) the unprocessed string of classes, periods included ".one, .two, .three"
• (better) a string with class names separated by spaces, like you would use in an HTML class attribute, "one two three"
• (good) an array of class names ["one", "two", "three"]

I also noticed that the id field keeps the trailing comma if something appears after it, which is a bug.

If this is something you're willing to change, I would be willing to work on the PR. Thanks!

Solution

Proposed changes:

• :foo{.one, .two, .three} should have { class : ["one","two","three"] } instead of { class : "one, two, three," }
• :foo{#id, .one} should have { id : "id" } instead of { id: "id," }

Alternatives

see above

[wooorm]

Why are you adding commas in there, they aren’t supposed to be there: the stuff between curly braces are HTML attributes, with the addition that shortcuts for ID and class exist: https://github.com/micromark/micromark-extension-directive#syntax

[wooorm]

I find no reference of commas, and do find many references w/o them, in the proposal: https://talk.commonmark.org/t/generic-directives-plugins-syntax/444

[benrbray]

Sorry, you're absolutely right about the commas, I was confused. The suggestion about returning the list of class names as an array still makes sense, though, I think. It would be a breaking change, so maybe it's not worth it when a .split(" ") will do.

[wooorm]

The downside of that is that I think it would be confusing to explain:

• Why class is split in cases such as class="a b"
• Why other space separated lists (according to HTML) are not split, in cases such as for="c d"

There are also a couple of other things that could be “smarter”, such as “boolean” attributes (download), so without a value, or style as an object. However, I think that would make it all a bit hard to explain. I’d say sticking as close to HTML is the most reasonable solution

[wooorm]

I don’t think it’s a wise idea to treat class differently, as stated in my previous comment, so I’ll close this.