doc
doc copied to clipboard
Revise single-word L to /type/x for errors
The problem
In d8c2b04256fb3b7727742e97dcbc8e2997b87af0, all L<x>
were changed to L<x|/type/x>
. This is OK as long as X is a capital letter, but it did not work if x was something else, like uniparse #2559
It was partially fixed here fe72b230390122794244ec7d380b22a8e8f415ba, but some errors remain.
Suggestions
We will have to revise both commits for errors such as the one pointed out above. And mainly not assume a particular prefix or accept these changes if they have not been tested before, as in testing if the newly generated link exists or not.
Previously, as seen in the Wayback machine (check say and gist), lower-case "bare" links were converted to /routine/x
apparently. I can't figure out where, or if it actually used some kind of registry and disambiguation to look this up. So it might be that it was wrong before the two big changes, and the second one is done according to spec, but I really have no idea.
My preference, right now, is that these kind of changes stopped until #2529 is done, if not until #2549 or even #1823. I'm not saying these two changes should be reverted, because the truth is that we don't why or if it worked to start with, because #2529. But there are lots of things we don't understand about the HTML generation from sources, so it's either build from scratch (with tests, of course), or a phased transition from the old to the new (which obviously needs tests too).
Besides, there's been no test for avoiding L<>
links in the future, and whatever code generated that is still there (and I can't figure out where it is).
OK, I found the code:
https://github.com/perl6/doc/blob/fe72b230390122794244ec7d380b22a8e8f415ba/lib/Pod/Htmlify.pm6#L58-L61
It really uses the same (I guess) heuristic that @cfa used in fe72b230390122794244ec7d380b22a8e8f415ba, which is lower-cap is routine (except for one case where it's substituted by syntax). So there was a bug there to start with. Now the thing is, do we want to auto-magically generate links to classes, as suggested here? To everything? To nothing? Any answer to this needs corresponding tests, of course.
CC @finanalyst.
Happy to revert both commits if that's the consensus, though there are issues with the heuristic listed above. Consider the following mappings:
-
atomicint
→/type/atomicint
-
class
→/syntax/class
-
close
→/routine/close
If these were written as L<atomicint>
, L<class>
and L<close>
, â…” of the generated links would be broken. Awareness of when implicit resolution occurs places an additional burden on doc authors and strikes me as error prone.
In general, explicit links seem more robust and easier to reason about in tests and tooling; this is orthogonal to overgeneration of /type
links in d8c2b04. Also, FWIW: fe72b23 guarded against rewriting /type/int
* and /type/atomicint
as routines (hopefully no instances were overlooked, anyway).
@JJ: did you have specific examples (or tickets) in mind re: "It was partially fixed here fe72b23, but some errors remain"?
Not really. The way implicit URLs were expanded was a bug to begin with, as you point out. #2559, linked above, lists a couple of examples, but there are bound to be more. #1433 has been there for a long time, and I've given it high priority, so that we test all local links in HTML generated pages. Explicit links are only as robust as the underlying logic. If implicit links are generated with the same logic as the content generation follows, they're just as safe and keeps us from breaking links if the underlying logic is changed. However, in this case
- Heuristic that generates links from bare words did not follow the page-generation logic.
- Expanding the links means it will fail if page-generation logic changes.
- Expansion following the (assumed) page generation logic failed, and fixing it failed in the same way as the original URL expansion (linked above) did.
Initially, I favored eliminating all kind of implicit keyword-to-URL expansion. But I'm not so sure now. Since the generation of links from path is ad hoc, any change in that logic will imply the rewriting of lots of internal links. Just imagine that all of a sudden we decide to change type to class|role. So, the thing is we need to stop assuming how htmlify.p6 works and really start writing a test suite that reverse-engineers all specs and can be used to refactor it (#1823 ) and from then on start taking decisions on what to keep and what to ditch, and how to move gracefully from one to another. So maybe reverting the two commits would make sense, but on the other hand both commits have helped us to find an error, and we can fix the links now that they are explicitly expanded, while it would have been impossible with implicit expansion. So I think it's better to leave them how they are, fix generated issues, and not lose sight of the big picture, which is the next generation of doctools on which @finanalyst is working.
Again, it's not clear to me that #2559 is a bug at all. Even if it is, it's unrelated to expansion of implicit routines, no?
Re: refactoring, I can see the argument there (though if our rules for determining routines/methods/declarators etc. are imperfect I fear it's moot—structural substitution across the doc
tree seems more robust and subject to auditing).
PS—Big 👍🏽 to #1433.
You are right. The original path-sep was simply an error, was not part of the implicit path expansion
Not even the other method indicated. There must be some other somewhere that made me think this.
So it might be the case that all original implicit links were right, your substitution of type by routine was right too, and this might only be an error that has been attributed to the wrong thing again but that has revealed a set of bugs anyway...
I don't believe this is currently an issue, and we have several tests for checking link validity (both in docs on on the website), and that links to types are valid. Closing ticket.