gjtorikian / selma

Selma selects and matches HTML nodes using CSS rules. Backed by Rust's lol_html parser.
MIT License
56 stars 3 forks source link

Support for writing attributes without a value, e.g. `element#tag_name["data-foo"] = true` #38

Open grekko opened 10 months ago

grekko commented 10 months ago

I'm using https://github.com/gjtorikian/html-pipeline to transform HTML output. AFAIU my request is about Selma and not html-pipeline, though. Please excuse any misunderstanding on my end and feel free to close this issue if I got things wrong.

For reasons I need to annotate certain HTML-tags with a a data-attribute, so I want e.g. <p>Foo</p> to be transformed to <p data-this-is-special>Foo</p>.

I am using the following custom NodeFilter-code for this:

class HTMLPipeline::NodeFilter::MyFilter < HTMLPipeline::NodeFilter
  def handle_element(element)
    element["data-this-is-special"] = true
  end
end

which raises an error:

`[]=': no implicit conversion of true into String (TypeError)

At this point I could not find any way to just add the data-attribute but have to set an explicit String-value for the attribute.

Would you be open to support – or is it even possible easily to do so – an API that allow to set an attribute without a value?


Added:

class HTMLPipeline::NodeFilter::MyFilter < HTMLPipeline::NodeFilter
  def handle_element(element)
    element["data-this-is-special"] = "true" # this works of course, so this my request is merely for cosmetics
  end
end
gjtorikian commented 10 months ago

In short, yes I agree with this change, although I am not sure what the syntax should be.

You're right that this is a selma change (which for some reason, I cannot transfer this issue), but deeper than that, selma is bound to the API of lol_html, which requires a string value. Right now I would guess that you can get away with element["data-this-is-special"] = "", and I don't disagree with what I think you're saying, which is, that looks ugly.

So what would you like the method/feature syntax to look like?

element.add_attribute("data-this-is-special")

Transparently, I can have this set an empty string to that attribute. The resulting HTML will look like this:

<a data-this-is-special="">

Is that okay, or would you prefer <a data-this-is-special />? (That would definitely need a change in lol_html but it would be a minor one.)

grekko commented 10 months ago

Thanks for the nice response!

Your proposal of the following API looks good to me!

element.add_attribute("data-this-is-special")

Is that okay, or would you prefer <a data-this-is-special />? (That would definitely need a change in lol_html but it would be a minor one.)

In an ideal world I'd rather have just <a data-this-is-special /> than <a data-this-is-special="" /> because it is easier to read.

But since Christmas is just over and the feature has to be provided by lol_html I'll have to wait for this 🤔

I'll open an Issue on the lol_html repo asking if this is something that someone could implement. Unfortunately I have no knowledge in rust and will have to depend on someone else to implement this.


Here the Issue on the lol_html-repo https://github.com/cloudflare/lol-html/issues/205