ex_doc
ex_doc copied to clipboard
Warn on invalid format inside backticks in links
[old Erlang child specification](`t:supervisor.child_spec()`)
will generate
<a href="`t:supervisor.child_spec()`">old Erlang child specification</a>
I find myself making this kind of mistakes, and ExDoc not warnings about it. I think if Exdoc ends up generating a link that starts with a backtick, it should emit a warning.
~Well. Actually i think the format should be supported to avoid this kind of mistake, and render it without the backticks.~
We should warn for invalid syntax indeed.
After years of working with ExDoc and Elixir, and being heavily involved in documentations, I have problems we this feature.
The problem mentioned above it is unrelated to the use of the backtick, it is because I was missing a colon, it should have been t:: with a double colon because the Erlang module is :supervisor
[old Erlang child specification](`t::supervisor.child_spec()`)
I tried different combinations, and only number 7 prints out the intended URL, and none of the others emit a warning.
- 1 [Text](t:supervisor.child_spec/0)
- 2 [Text](t:supervisor.child_spec())
- 3 [Text](`t:supervisor.child_spec/0`)
- 4 [Text](`t:supervisor.child_spec()`)
- 5 [Text](t::supervisor.child_spec/0)
- 6 [Text](t::supervisor.child_spec())
- + 7 [Text](`t::supervisor.child_spec/0`)
- 8 [Text](`t::supervisor.child_spec()`)
There may be room for improvement.
What I would expect is ExDoc to emit a warning in every case and suggest a solution to correct the problem if possible. ie. to add backticks, to add missing color, to replace arguments with arity
I would start with backticks only and move forward with more warnings if no false positives show up.
+1 for emitting a warning and NOT generating the link, returning a href with backticks is obviously a bug. Let's ship that and then in the future we can iterate on it, trying to be more helpful.
Hey! I did some experimentation with this and I think one approach for this case would be:
- If we fail to parse something inside backticks as a reference, we warn it as an invalid reference and remove the link from it.
I think this will work fine because as I understand, if we cannot find parse something as a reference, we assume it to be a link and just return it inside the link markdown block, but since backticks surrounding links are invalid CommonMark, we can check for their presence and emit an "malformed expression" warn. Any thoughts?
Also I've noticed that invalid mix tasks are warning but not removing the link (e.g: [foo](`mix foo`) == <a href="`mix foo`">, is this the expected behavior? If not, I'm willing to open another pull request for removing the link on invalid mix tasks before implementing this one.
Do we remove the link in other cases? Or is the proposal to always remove? That also works for me but harder to discover (a broken link will eventually break).
agree on warning when there are backtics and they don’t point anywhere.
Do we remove the link in other cases? Or is the proposal to always remove?
Currently only links for hidden functions are removed.
From what I understand the idea is to never emit links with backlinks, since they'll always be broken.
What is currently causing them to be emitted is the fact that whenever we find something under backticks and fail to parse it, we return it as a link, thus creating the broken links. Thus I think an invalid reference inside backticks would fall into an "invalid/malformed reference" category, and we could warn and remove the link since it would be generated with backticks.
Sounds good to me!