Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
grekko opened this issue Jan 6, 2024 · 2 comments

Comments

@grekko
Copy link

grekko commented Jan 6, 2024

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
Copy link
Owner

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
Copy link
Author

grekko commented Jan 8, 2024

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 cloudflare/lol-html#205

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants