9.10a3 General Purpose Block Feedback

[facelessuser]

Alpha 3

Documentation: https://facelessuser.github.io/pymdown-extensions/extensions/blocks/

The purpose of this discussion is to allow feedback for the latest iteration of General Purpose Blocks 9.10.a3. It should be noted that any and all feedback is welcome, but the current focus is on the API for creating block plugins.

Changes in Alpha 3:

• NEW: General blocks now use an indented option block right after the header. yaml_indent option has been
  removed.
• NEW: Added new "Definition" block that allows the creation of definition lists.
• NEW: Simplified argument configuration.
• NEW: Some internal cleanup.
• NEW: Documented current API.

Feedback Needed

Syntax

We still have the following open syntax question. Currently, we have one general option that is only available during the alpha (and maybe beta) stage. By the final release, we'd like to remove this option with a permanent solution.

1. Which is preferred, syntax with ::: or ///? Currently, the option colon_syntax allows selecting between the current default syntax of /// and the colon syntax (:::). Which is generally preferred by the community?

API

We are looking for general feedback on the API.

1. Does this look sufficient or are we missing some cases that adjustments to the API could help better handle?
2. Are there any confusing aspects of the API? Do we need more clarification or even more simplification of certain parts of the API?
3. etc.

[waylan]

• Added new "Definition" block that allows the creation of definition lists.

Didn't see this in #1883 until today. So I'm following up here. I find this a weird one because there is an established (and widely used across implementations) definition list syntax. Of course, it isn't supported everywhere and I know some don't care for it. And anyone is certainly free to define their own syntax with their on extension. I just find it odd to include this in the default set of supported blocks. I would expect this to be separate. But maybe that is just me. I have nothing more to say about it. Of course, this is your project so you can do what you want.

[facelessuser]

This is new, and I am aware that some parsers respect definition lists as Python Markdown defines them, but there are certainly many that do not. I know in my editor it is constantly thinking that additional paragraphs under a definition are code blocks. It is kind of annoying because they aren't widespread enough.

Indeed, no one is required to opt into all the blocks that will be provided. We can certainly discuss defaults as well. In this case, I imagine that what we've implemented can coexist with definition lists in the sense that if you do not use them both (physically use both syntaxes) that having them both enabled should cause no trouble.

The only conflict you could possibly have is if you are physically defining general block definition lists, but then also have the original "definition lists" enabled and follow up with an indented block. In this case, I could see the original definition lists trying to append it to the description of the <dl> element right before it, but then I'd ask, why are you defining general block definitions but also enabling the alternative form of definition lists. In this case, I'd recommend excluding the original "definition lists".

/// define
term

- description
///

    some block

Again, one of the reasons I have an interest in this plugin is to eliminate non-standard indentation syntax from my own documents but is not something I am forcing on others.

[facelessuser]

It may make sense to maybe make most or all opt-in. Maybe only include the generic HTML block by default and all others are added to it...

[waylan]

It may make sense to maybe make most or all opt-in. Maybe only include the generic HTML block by default and all others are added to it...

This may be a good way to start. Then, over time with more feedback, it may be clear that users are always using some block types more regularly. If so, they could be included by default in a future release.

[waylan]

The API looks pretty comprehensive. After an initial read through the documentation, here are my observations.

I'm struggling to see the difference between the on_init and on_parse events. Obviously, on_parse can return False which causes the block to fail (btw, what happens when it fails?). Other than that, they seem to be identical. My best guess is that perhaps the args and options available in on_init are the default options for the extension whereas the args and options available in on_parse include the args and options pulled from the block itself. If so, that was not clear to me from the documentation. I am only guessing that because the name on_init suggests parsing hasn't happened yet which means the args and options haven't been retrieved from the block. If that is not the case, then I would suggest consolidating the two into on_parse.

The on_markdown event also confused me at first. Then I realized it was returning a string (-> str:). Perhaps the first column of the table should be labeled return values (or similar) rather than options. Options sound like something that should be passing in, not returned. And perhaps more explicit text in the description would help here as well.

A minor issue, but the type_string validator is defined twice in the documentation. Once before and once after type_insensitive_string.

I find it curious that the type_html_attribute_value (and I assume the other validators which act on HTML attributes) escape the values as that happens by the serializer after the etree is complete. I realize that the serializer is smart enough to avoid escaping existing entities (> does not become >), so this should not break things, but why not let the serializer take care of this?

Finally, wow that is a lot of validators. At first I was going to object as YAML already has types, but now I see you are doing a lot more than differentiating between strings, integers, and bool values. This is extremely powerful. I'm not certain all of them will be widely used, but they exist now so no reason to remove them. And of course, as users can define their own, you are not limiting users to the default set. Two things I don't see in the documentation are any direction on how to create your own validator and the import path of the existing validators.

Overall I think this looks like a good API. However, I haven't looked at the code or attempted to use it yet. So my impressions could change. But I'm not sure when I'll have time for that.

[waylan]

Ah, so on_init runs once per document, but on_parse runs once per block. Is that it?

[facelessuser]

Sort of, but not quite. We kind of need an instance per block we run into. Since these can be nested, you could have multiple blocks in different states. So, we instantiate a new block every time we encounter a generic block. on_init is meant for global options and initializing the block to some sane default state. on_parse is where the per block options and arguments are gathered allowing you to validate them and make decisions based on them, potentially updating class variables and such to enable/disable features.

[facelessuser]

I've cleaned up documentation related to on_init, on_parse, and on_markdown. No functional changes have been made at this time.

[waylan]

That clears things up. Now I understand. I would suggest putting a polished version of that explanation in the documentation somewhere. It did not occur to me that each block get's its own instance of the class. Although, in retrospect, some of the documented behaviors only make sense with that behavior.

[facelessuser]

Done

[facelessuser]

I'm thinking about changing the argument delimiter from | to just /. Anyone have strong objections or other suggestions?

The only reason I'm considering this change is that some syntax highlighters can think that the declaration is the start of a table and can affect some highlighting afterwards. I haven't found any actual Markdown parsers that get confused by this, but I'd like the final syntax to be as un-intrusive as possible.

So, as a summary, the change would be changing this:

/// block | argument

to

/// block / argument

Again, I'm open for people to suggesting something different, but the main options I see are encapsulating the block name with something (quotes, brackets, etc.) or using some delimiter. I think I'm okay with just changing | to /.

[waylan]

Sigh! I really dislike this change. The conflict with highlighters is unfortunate. It would be preferable to design a syntax without consideration to outside forces like this. But we don't get to work in our idealized world. Unfortunately, I don't have any suggestions for a workaround.

[facelessuser]

It's just something I'm considering. I'll think about it. My preference was always |. And the annoyance in my editor is very minor. I posed this as a question as I suspect that there may be some dislike.

[facelessuser]

I played with this more. It's an odd case in my editor, that requires a very specific case as most cases it does not trigger. So I'm going to not make this change, especially for such a minor cosmetic change that, even in my case, is very finicky to reproduce.

[facelessuser]

The next version will be Beta 1. We've cleaned up some API things and documented some missing parts. Assuming no issues, I plan to release generic blocks to the public in an official release, but marked as "experimental".

Some changes coming:

1. We are removing colon_syntax option and moving forward with the /// syntax.

2. We've revised the HTML block to accept Emmet style syntax for specifying tags with attributes.I updated my documentation using the new general blocks in most places. I found it made specifying tags less of a chore when using the block. It made sense to streamline it for a block that is focused on creating HTML blocks.

   /// html | div#id.class[name=value]
   content
   ///
   

3. Specifying attributes on other non-HTML specific blocks was a bit laborious as well, so we shortened the attributes to just $ as it will likely be common to use, and less typing is preferred, at least to me. Also, $ takes attributes the same as HTML blocks.

    /// admonition | Title
       $: .warning#id
   content
   ///
   

4. We also dropped the type option on Admonition and Details as it was kind of redundant as you can just use $ to set classes.

Feel free to get in your last remarks, opinions criticisms. I hope to create the beta either today or tomorrow. Final input will be considered for the official release.

[waylan]

what actually happens is that they check if the block manager is loaded, and if not, they register it and then register themselves within the manager. This requires the developer to copy and paste this logic if they want to define their own block plugin.

Actually, a base class could implement this so that any subclass would get this behavior for free.

What we have now is much easier on the developer, and only slightly awkward for the user. I'm not sure if it is worth the effort to try and restructure this.

I have found that the best approach is usually to give preference to the user in situations like this. That would suggest that the effort would be worth it to the users. My concern is that it is currently overly complex to get anything working at all. Refactoring so that each block type was its own extension which appears to stand alone would greatly simplify this for the user.

That said, I am viewing this from the perspective of someone who has only read the documentation. I still have not looked at your code at all and am not clear on how much work it would be to do this refactor. I also understand that this has gone on for some time with multiple refactors and you are understandably reluctant to do any more.

[facelessuser]

I can see how complicated it might be. If it isn't too bad, I might give it a go. I can understand the desire to a more simple interface.

[facelessuser]

I think we may be able to subclass the extension class that handles the registering of processors. If we did that, we can probably drop the on_register event...I feel like there may be some complexity with reset events...I guess we'll see.

[facelessuser]

Okay, I got block extensions registering on their own. Moving forward, if you want to you'd like to create one these block extensions, you'll have to use the pymdownx.blocks.BlockExtension instead of markdown.Extension.

Instead of overriding extendMarkdown, you override extendMarkdownBlocks. That method receives md and blocks. blocks is used to register block plugins while md can be used to register normal extensions. We handle the abstraction in extendMarkdown which should not be overridden directly. Everything else is like usual.

class AdmonitionExtension(BlocksExtension):
    """Admonition Blocks Extension."""

    def __init__(self, *args, **kwargs):
        """Initialize."""

        self.config = {
            "types": [
                ['note', 'attention', 'caution', 'danger', 'error', 'tip', 'hint', 'warning'],
                "Generate Admonition block extensions for the given types."
            ]
        }

        super().__init__(*args, **kwargs)

    def extendMarkdownBlocks(self, md, blocks):
        """Extend Markdown blocks."""

        blocks.register(Admonition, self.getConfigs())

I think this is definitely for the better. Now I have to update all the tests and update the docs 😢.

[facelessuser]

Everything is working, but we'll have to delay the beta until the end of the week as I don't feel like getting it all polished up today. But I think we are getting close to the finish line.

The plan is to release the beta. People can try it out and report any bugs. Feedback is still welcome, but I will be more critical of big changes moving forward. Then we'll go live in a non-beta release adding a disclaimer that these are "experimental". Assuming they are generally well received, we can remove the disclaimer at some point.

[facelessuser]

Ended up getting the beta out. The new discussion is here: #1961.

Now's the time to get your feedback in before this goes public.

Locking this discussion in favor of the Beta 1 discussion.