Tune attr list regex

[waylan]

Ignore empty braces. Braces must contain at least one non-whitepsace
character to be recognized as an attr list.

Attr lists for table cells must be at the end of the cell content and must
be seperated from the content by at least one space. This appears to be
a breaking change. However, it is consistent with the behavior elsewhere.

Fixes #898.

[waylan]

@ryneeverett you had implemented the original fix for table cells in #267. Do you (or anyone else) have any objections to the breaking change here?

I still need to add the release notes on this, but am waiting for feedback first. The commit message (first comment) sums up the changes.

I also need to document the table cell behavior in the docs. I don't believe it is mentioned at all. That could give more weight to the breaking change.

[ryneeverett]

I wouldn't have any objection to the breaking change if it resulted in a better syntax and I do appreciate that you're more fully specifying behavior here, but having the absence or presence of a single space determine behavior strikes me as obnoxiously subtle.

What if there were another special character within the brackets to determine whether it applies to the content of the cell or the element itself? For example, what if a caret indicated that it was to apply to the wrapping element: {^.foo} => <td class="foo">.

[waylan]

having the absence or presence of a single space determine behavior strikes me as obnoxiously subtle.

Perhaps, but that is already how it works everywhere else. Take this paragraph below, where the list is in the middle of the paragraph and therefore not an attr list of the paragraph.

Some text in a *paragraph* { .foo } that includes what looks like an attr list in it.

Note that the item directly before the non-attr list is an em element. However, because of the space between the two, the list will be ignored. If we were to change that behavior, we would need to change it across the entire extension.

The way I see it is that we are actually making table cells behave like the rest of the extension. In fact, with this change, the code which handles headers (which also requires the attr list to be on the same line) is the same code which handles table cells. The only change was to add support for td and th as parents.

what if a caret indicated that it was to apply to the wrapping element: {^.foo}

That is an interesting idea. However, it seems to me that if we were to do that, then it would also be required for paragraphs, headers, ect. Perhaps instead that would make sense as a way to apply attrs to a parent element. For example, on a list item, {^ .foo} might be applied to the ul element rather than the li itself. In a table cell, it might be applied to the tr and so on.

We have certainly received requests for such a feature in the past. Personally, I'm not really interested in implementing it, But I know many users would use it. Regardless, I see that as a separate issue from this one. If any one wants to discuss that further they should do so elsewhere. I simply mention it here as a means to explain why I don't think that syntax works for this use case.

[ryneeverett]

However, because of the space between the two, the list will be ignored. If we were to change that behavior, we would need to change it across the entire extension.

I find it less bothersome that the the presence of a space breaks functionality than that it would add functionality. If there can never be a space before an attr list that's a fairly easy syntax rule to visually inspect -- not so if there should sometimes be a space before an attr list.

That is an interesting idea. However, it seems to me that if we were to do that, then it would also be required for paragraphs, headers, ect.

Couldn't one say the same for the way you're using the space character to indicate the element? Does that also work for paragraphs?

Perhaps instead that would make sense as a way to apply attrs to a parent element.

I agree that we shouldn't get too deep into this proposed feature here, but it's relevant that such a feature might not play well in combination with the syntax supported by this patch. For example, what happens when you have content{^.foo}? (There's no space before the attr list there. I feel like I need tell you that because it's a poor syntactic distinction. :) )

(By the way, I admit that my original implementation is inconsistent and I don't care to defend it.)

[waylan]

@ryneeverett I understand your objection. However the same objection could be made for headers. Consider this:

# Header 1 with *emphasis* { .foo }
# Header 2 with *emphasis*{ .foo }

Of those two, the first will have the foo class assigned to the h1 element (because of the space), but the second will have the class assigned to the em element (because there is no space). The extension has always worked this way. I am simply extending the same behavior to table cells. If we are going to change the behavior, then we need to change it everywhere.

The problem is that the existing behavior is based upon other implementations which came before it. And I intend to maintain compatibility with those other implementations (therefore we are not changing the global behavior). Of course, as tables are a non-standard feature of Markdown, those other implementations don't specifically support table cells. However, for consistency within our implementation, we should use the same behavior for table cells as exists for headers (or any other content which must remain on a single line in Markdown).

If the only objection is regarding the required space, then I think I am ready to move forward with this.

[ryneeverett]

Agreed, this does seem to be the only option that is both backwards compatible and consistent. Further improvement might be outside the scope of the built-in extensions.