Conversation
wilzbach
left a comment
There was a problem hiding this comment.
I am not very familiar with the DDox codebase, so my first review might not be that helpful..
views/ddox.docpage.dt
Outdated
| ul | ||
| - foreach (i, dg; info.docGroups) | ||
| - auto itm = dg.members[0]; | ||
| li: a(href="\##{i}") #{dg.kinds.joiner("/")} #{itm.nestedName} |
There was a problem hiding this comment.
How about using itm.nestedName.slugify?
A not optimized, but nicely working slugify method that I copy/paste around from time to time:
string slugify(string target) {
import std.regex;
import std.string : toLower;
static immutable slugR = ctRegex!(r"[^a-z]+", "g");
auto ret = target.toLower.replaceAll(slugR, "-");
if(ret[0] == '-') {
ret = ret[1..$];
}
if(ret[$-1] == '-') {
ret = ret[0..$-1];
}
return ret;
}There was a problem hiding this comment.
That would possibly result in duplicate anchor names. Using the mangled name as a basis could work, but the question is if that would be much better, since any small change in a function or template signature would change the outcome (thus rendering permanent links nearly as useless as for simple indexes).
views/ddox.docpage.dt
Outdated
| - foreach (i, dg; info.docGroups) | ||
| - bool got_function = false; | ||
| - int hlevel = 2 + multi_page; | ||
| - auto kinds = dg.kinds; |
There was a problem hiding this comment.
Is this really worth it?
(kinds is not used much below)
There was a problem hiding this comment.
It's used once in the beginning and once in each foreach iteration. Usually the number of elements that kinds operates on will be quite limited, but it performs a GC allocation and sorting, so it isn't really cheap. Especially the GC allocation can be significant (it should really be eliminated completely, but as long as there are others this doesn't have the highest priority).
This should be more obvious from the name, though. I'll rename kinds to collectKinds.
| - if (info.docGroups.length > 1) | ||
| h2 #{item.kindCaption} #{item.nestedName} | ||
| - if (multi_page) | ||
| h2(id=i) #{kinds.joiner("/")} #{dg.members[0].nestedName} |
There was a problem hiding this comment.
If you go down the slugify road it would be sth. like: h2(name=dg.members[0].nestedName.slugify)
views/ddox.docpage.dt
Outdated
| - if (hlevel == 2) | ||
| h2 #{item.kindCaption} #{item.nestedName} | ||
| - else | ||
| h3 #{item.kindCaption} #{item.nestedName} |
There was a problem hiding this comment.
Why don't you use the new heading function here?
There was a problem hiding this comment.
Forgot to change this when I introduced heading later in the process. Will fix.
|
Okay, I think I have everything and will squash and merge now. Would still be interesting if there is a better (more stable) way to generate page anchors. Maybe something in-between mangled names and indexes, or maybe something that just involves the base kind (function/class/...), the case sensitive name and parameter names. |
This is a more complete approach for #146, combining all declarations in a doc group and removing several redundant sub headings. Also reduces the footprint of the legal footer.
567956e to
5f7aef8
Compare
I highly doubt that it will matter much if the link is "broken" as the user is already on the single artifact page. |
|
You are probably right. I'll still open a ticket, but leave that topic for later. |
This is a more complete approach for #146, combining all declarations in a doc group and removing several redundant sub headings. Most importantly, it behaves correctly for doc pages with multiple doc groups.
With these changes, it now looks like this:
