puppetlabs / puppet-strings

The next generation Puppet documentation extraction and presentation tool.
http://puppetlabs.github.io/puppet-strings/
Apache License 2.0
90 stars 84 forks source link

Anchor links in generated markdown are not unique #300

Closed danielparks closed 2 years ago

danielparks commented 2 years ago

Describe the Bug

The anchor names in markdown generated documentation for a module are not unique. This is a particular problem for attributes such as ensure. Each attribute with the same name is given the same anchor link name, which breaks linking.

For example, see the docs for my rustup defined type. The ensure attribute links to #ensure, but the browser (Safari, in my case) can’t distinguish between the ensure attribute for the rustup::global class and the rustup defined type.

You can also conflicting anchor names for types as well, since it just ignores characters like :.

Expected Behavior

All anchor names should be unique.

Steps to Reproduce

❯ cat >test.pp     
# @summary test::ab class
# @param ensure attribute on test::ab
class test::ab ( $ensure ) {
}

# @summary test::a::b class
# @param ensure attribute on test::a::b
class test::a::b ( $ensure ) {
}
^D
❯ puppet strings generate --format markdown test.pp >/dev/null
❯ fgrep name= REFERENCE.md                                
### <a name="testab"></a>`test::a::b`
##### <a name="ensure"></a>`ensure`
### <a name="testab"></a>`test::ab`
##### <a name="ensure"></a>`ensure`

Environment

binford2k commented 2 years ago

Hey buddy!

Interestingly, the anchors appears to be created at https://github.com/danielparks/puppet-rustup/blob/f405dac71b966d875b0d2e96984b0a2166032fcf/REFERENCE.md#ensure-1, but the link to it doesn't keep the index

danielparks commented 2 years ago

Hey Ben! Been awhile!

I think that’s an artifact of GitHub generating the HTML. Looks like it appends a number onto each successive anchor, but as you say, it doesn’t fix the links (presumably it would have to guess).

Looks like Forge does basically the same thing, though it also doesn’t fix the links.

Probably it makes sense to look at the underlying markdown: https://raw.githubusercontent.com/danielparks/puppet-rustup/f405dac71b966d875b0d2e96984b0a2166032fcf/REFERENCE.md


I actually fixed this (mostly) with post-processing: https://github.com/danielparks/puppet-rustup/pull/15/files#diff-1aacbb5834541d8fb2daa76bafbe0339f4f637bdea56c4c6411fe5d29c603995

In retrospect, maybe the issue “anchor links not unique” is too large — to deal with all the (uncommon) cases you basically have to construct a big index as you go and make sure you’re referring to the right one — or, make some sort of reversible encoding that supports whatever characters are allowed in an anchor.

The script I linked above is a giant hack but it probably deals with the vast majority of cases that don’t involve type names like test::a::b and test::ab. All it does is:

  1. Construct anchor names to parameters including the type, e.g. my::class::$param (though as you can see it does it incorrectly… oops).
  2. Replace each invalid character with _, e.g. my__class___param.

You can see the results in GitHub HTML and raw Markdown.

danielparks commented 2 years ago

I made a few improvements to my script. I’m pretty sure it’s impossible for it to generate non-unique anchor names, but I’m not 100%.

chelnak commented 2 years ago

Fixed in #300 - Thank you @danielparks