Fix the "converters" header link in the README

[a2wd]

Added an anchor in the converters section heading for the link from the encodings section to point to (previously, the anchor tag/name attribute didn't exist so the link didn't do anything)

[FagnerMartinsBrack]

I don't understand what this do. All headers in Github markdown have links Can you please elaborate or record a video showing the difference? Thanks.

[FagnerMartinsBrack]

Closing due to the lack of feedback from the OP. If you want to continue the discussion, please comment here mentioning one of the project members.

[a2wd]

@FagnerMartinsBrack sorry for the delay, I've added a screengrab (below) unless it gets compressed, full size you should be able to see the difference - the left is the version in this PR, the right if the current master.

[FagnerMartinsBrack]

Shouldn't it be #converters with an s in the end? See https://github.com/js-cookie/js-cookie/tree/b1f0a45ac82c892442215fd3cc54877465f3ae8b#converters

[FagnerMartinsBrack]

Probably the fix is in the link, not the header. The link should point to #converters instead.

[FagnerMartinsBrack]

It looks like this typo has existed since June 2016 when it was added. See the "Encoding" section in the context of that revision.

[a2wd]

In that revision and previous ones I looked at it doesn't appear to have an anchor or name. The github markdown generator does add a custom #user-content-converters anchor nearby the heading though, but presumably that's not what you want to link to?

[FagnerMartinsBrack]

In that revision and previous ones I looked at it doesn't appear to have an anchor or name.

Maybe Github does some magic, the important thing is that it works even without an id in the header element:

(Don't ask me why)

The github markdown generator does add a custom #user-content-convertors anchor nearby the heading though, but presumably that's not what you want to link to?

That's not what we should link to. That's Github generated markup. We shouldn't be adding to the project's Markdown level something that is of knowledge of Github only. Github can change the id="user-content-converters" at any time, the raw HTML is strictly internal behavior of Github, and it has the same problem as when we rely on library internals.

In this case, the best approach is to simulate the same link that Github adds when you click on the anchor in the left side of the title, see the image below:

That link points to https://github.com/js-cookie/js-cookie/tree/4e77d05b59692b664fe869f3e2dd54b77c13badf#converters

The likelihood of Github supporting their header generated anchor format is much higher than supporting a consistent generated HTML markup. Since they don't explicitly document that anchor and how link to them (or at least I'm not aware of any docs about it) this is still internal, but it's using an internal that if changed will also break several other projects out there. For that reason, it's safer than inspecting the HTML markup and linking to a custom anchor tag like #user-content-converters.

The <a name="converter"></a> is also a good approach because it ensures we have control of the element id. However, it mixes HTML with markdown but no apparent benefit while also breaking consistency with the rest of the file.

Given the tradeoffs, fixing the line 145 to point to [converter](#converters) seems to be the best approach in this case.

Make sense?

[a2wd]

@FagnerMartinsBrack as discussed, I attempted with a change to #converters but that doesn't help either - you can see this at https://github.com/a2wd/js-cookie/blob/master/README.md#converters

[FagnerMartinsBrack]

@FagnerMartinsBrack as discussed, I attempted with a change to #converters but that doesn't help either - you can see this at https://github.com/a2wd/js-cookie/blob/master/README.md#converters

It doesn't work with your link because you have changes in the header where there's an additional space on it before the tag you have added in the markdown, so the header anchor is interpreted as #converters- instead of #converters. Here's the same link you posted but now working: https://github.com/a2wd/js-cookie/blob/master/README.md#converters-

By the way, according to the contributing guidelines, we require PRs to be open using a topic/feature branch instead of master. See this for additional info: https://help.github.com/articles/about-pull-requests/

[FagnerMartinsBrack]

As explained in the submitted review, if you remove <a name="converter"></a>, then the link will just work.

[FagnerMartinsBrack]

Ping @a2wd. Are you still looking into this?

[a2wd]

Again, sorry for the delay - this is now done and working 👍