search

mathjax / MathJax-node

MathJax for Node
Apache License 2.0
614 stars 96 forks source link

state.ID must be a number #427

Closed pkra closed 5 years ago

pkra commented 6 years ago

While it's not documented, we can set state.ID manually (though #426 makes this somewhat annoying).

Because of

https://github.com/mathjax/MathJax-node/blob/5f6cf587082681c3492df938a9e5347f10c68ca7/lib/main.js#L709-L710

state.ID must be a number.

While that's natural for automatic ID generation, it's limiting when using custom IDs. For example, a hash of the input string is sometimes a better fit for IDs when storing equations without duplicates (since duplicate IDs have to be handled manually anyway).

It seems like ID does not have to be a number anywhere else, so something like 

    if (typeof ID === 'number') ID++;
    else ID = 0;

(moved below the declaration of id) would allow more flexibility.

Would you be open to a PR?

pkra commented 6 years ago

If ID cannot be changed, then perhaps a new input option to specify an ID for the title/labelledby would be just as helpful.

dpvc commented 5 years ago

perhaps a new input option to specify an ID for the title/labelledby would be just as helpful.

That was going to be my suggestion as well. As I mention in #426, the state is not something that the user should be adjusting, as it is not a configuration object itself.

pkra commented 5 years ago

closing in favor of #429