search

markdown-it / markdown-it-footnote

Footnotes plugin for markdown-it markdown parser
https://markdown-it.github.io/
MIT License
210 stars 59 forks source link

Avoid footnote ID collisions across multiple documents #8

Closed sadlerjw closed 8 years ago

sadlerjw commented 8 years ago

Based on discussion in #7 - if env.documentId is supplied and is a non-empty string, footnote IDs are now in the form fn-some-document-id-1 / fnref-some-document-id-1. (See #7 for details)

To test, I needed a version of generate() from markdown-it-testgen that supported passing an env variable through to render. After discussion in https://github.com/markdown-it/markdown-it-testgen/pull/1, I inlined most of generate with the necessary modifications. (This also required me to add chai as a new dev dependency.)

puzrin commented 8 years ago

I don't like name documentId. Too long + camalcase (because consists of 2 parts).

sadlerjw commented 8 years ago

Oh okay, I thought that was your suggestion from #7, which I liked. docId is the obvious next idea, which still has two parts...after that, I'm not sure what else to name it. (Maybe baseName or something? Not sure I really like that either though...) Open to suggestions!

rlidwka commented 8 years ago

docId is the obvious next idea, which still has two parts...

I was asked for my opinion about this particular bike shed earlier today, so here it is: basename works just fine (I'm not really a fan of camelcase either), or maybe a prefix.

PS: Just out of curiosity, why are you using chai? It's an extra dependency, and built-in assert can do the same job (we're not running those tests in a browser as far as I know).

puzrin commented 8 years ago

PS. i use http://www.thesaurus.com/ to search words.

puzrin commented 8 years ago

Is it safe to use { id: XXX } ?

sadlerjw commented 8 years ago

@puzrin I think for this particular use case, anchorBase or hashBase is most descriptive, but maybe isn't a great name for something that might eventually mean something more generic in markdown-it core. I'm a little concerned with id because I'm not sure it's entirely clear what the id describes. I like basename a lot, although my personal favourites are documentId and docId. I'm happy to leave it to your judgment as maintainer what to settle on, including stuff I'm not a huge fan of :)

@rlidwka you're entirely right, I didn't know that (node newbie). I've removed the chai dependency.

puzrin commented 8 years ago

Let's wait a couple of days, to think about name.

PS. About chai - in testgen it was used to enable colored diffs at mocha output. But in single package that does not make sense.

rlidwka commented 8 years ago

Merged, thanks!

The property is named env.docId now.