jgm / djot.js

JavaScript implementation of djot
MIT License
141 stars 15 forks source link

question: Add class name to all sections #87

Open Brixy opened 4 months ago

Brixy commented 4 months ago

Hi all,

This is just a question. Sorry if this is not the right place.

Adding class names to all sections using a filter is no problem.

This is my current solution. But is there a more elegant/suggested way of doing so (apart from ternary if notation)?

return {
    section: (el) => {
        if (typeof (el.autoAttributes) === 'object') {
            el.autoAttributes.class = 'my-class';
        } else {
            el.attributes.class = 'my-class';
        }
    },
};

Thanks a lot!

jgm commented 4 months ago

I see that this is painful. We could perhaps always add an empty object for attributes instead of using null? Or export a function that allows setting an attribute value?

@matklad any thoughts?

matklad commented 4 months ago

Yeah, in my blog I have the following snippet (which is still on a very old version of djot):

function add_class(node: AstNode, cls: string) {
  node.attributes = node.attributes || {};
  const attr = node.attributes["class"];
  node.attributes["class"] = attr ? `${attr} ${cls}` : cls;
}

It deals with both attribute object being empty, and with the fact that adding a class requires string manipulation.

We could perhaps always add an empty object for attributes instead of using null?

I think this would be a clear win, yes. This means that we erase the syntactic distinction between no attributes and an empty attribute set on the semantic level, but I think that's correct. Djot's AST guarantees semantics, it doesn't guarantee round-tripping through original data.

Though, given than we now distinguist between auto attributes and normal attributes, maybe round-trippability is a goal now?

Or export a function that allows setting an attribute value?

For manipulating attributes, I think we don't need to do that, if we always set them to an empty object. Manipulating class list though additionally requires string splitting, and for that I think a bunch of helper functions would be nice.

If djot AST objects were proper classes then el.addClass|hasClass|removeClass would have been no-brainers. But our AST objects are just JSON without any methods, and that feels like the right design. So, addClass should live as a free functions somewhere. Which is perhaps fine? Funamentally, these helpers are just easy to maintain shortcut, so adding&maintaining them is low-cost, even if we later discover some better way to solve the problem.

Brixy commented 4 months ago

Thank you very much, guys. Your proposed solutions would simplify such use cases.