Improve ATOM constants naming

[Lonami]

The current naming is bad:

Constants

ATOM_LOCALNAME_
ATOM_LOCALNAME__2A
ATOM_LOCALNAME__6B
…
ATOM_LOCALNAME__73_74_72_69_6B_65_74_68_72_6F_75_67_68_2D_74_68_69_63_6B_6E_65_73_73

This would be a lot better:

Constants

ATOM_LOCALNAME_
ATOM_LOCALNAME__STAR
ATOM_LOCALNAME__K
…
ATOM_LOCALNAME__STRIKETHROUGH_THICKNESS

Having to do this isn't pretty :)

use html5ever::{
    ATOM_LOCALNAME__61 as TAG_A, ATOM_LOCALNAME__62 as TAG_B,
    ATOM_LOCALNAME__62_6C_6F_63_6B_71_75_6F_74_65 as TAG_BLOCKQUOTE,
    ATOM_LOCALNAME__63_6C_61_73_73 as ATTR_CLASS, ATOM_LOCALNAME__63_6F_64_65 as TAG_CODE,
    ATOM_LOCALNAME__64_65_6C as TAG_DEL, ATOM_LOCALNAME__65_6D as TAG_EM,
    ATOM_LOCALNAME__68_72_65_66 as ATTR_HREF, ATOM_LOCALNAME__69 as TAG_I,
    ATOM_LOCALNAME__70_72_65 as TAG_PRE, ATOM_LOCALNAME__73 as TAG_S,
    ATOM_LOCALNAME__73_74_72_6F_6E_67 as TAG_STRONG, ATOM_LOCALNAME__75 as TAG_U,
};

We could go further and use naming like TAG_A, TAG_B, but this gets tricky if there are attributes (not tags) that share names with tags. Maybe just ATOM_A? The LOCALNAME feels redundant.

[Lonami]

If backwards-compatibility is a concern, I would suggest adding a module html5ever::constants with all the ATOM_ constants.

[Lonami]

…and only after trying to solve this issue I found that string_cache_codegen generates a macro, which for html5ever is local_name!.

Well, that's a lot nicer to use, but at the very least, I would like to see the documentation for this crate to be improved in this regard (for now). This might mean adding a tiny guide on the main page of the documentation saying "when matching on tags, you likely want to use this".

Future work: that guide should show the basics on parsing HTML, which is what most people probably want to do.

[Lonami]

Maybe provide something like this, perhaps have the string_cache crate generate this?

macro_rules! make_tag_consts {
    ( $( $tag:tt => $constant:ident ),* ) => {
        $(
            const $constant: string_cache::Atom<html5ever::LocalNameStaticSet> =
                html5ever::local_name!($tag);
        )*
    };
}

make_tag_consts!(
    "a" => TAG_A,
    "href" => ATTR_HREF
);

Otherwise, users need to depend on string_cache just to refer to Atom, and typing TAG_A is arguably better than typing local_name!("a") every time.

Honestly I think the synonyms should be provided by html5ever directly.

[Ygg01]

@Lonami Hi. What are the benefits of using TAG_A instead of just local_name!(a)?

Using constants probably won't help with importing string_cache unless you never want to do anything with constants?

[Lonami]

Hi. What are the benefits of using TAG_A instead of just local_name!(a)?

It's semantically clearer what one means, since macros can do pretty much anything after all. But my original issue was that the constants were bad and I had no idea local_name! existed, so I think it's enough to better document the existence of such macros.

The current docstring for the macro reads:

Takes a local name as a string and returns its key in the string cache.

It's not very obvious (at least to me) that this can be used in the match arms when matching against tag names.

[Ygg01]

Yeah, I'm definitely for clearer docs.

And maybe allowing constants for easier reference is ok. That said. local_name is the correct name for HTML tag name, at least when namespaces are concerned.

I just don't see how exporting these constants will save you from importing string_cache?

[Lonami]

I depended on string_cache to be able to name the constants' type, so that I could define my own constants with better naming. However, I wouldn't need to do this if the html5ever provided the better (more reasonable) names itself.