D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 6017 - std.algorithm.remove has a wrong link
Summary: std.algorithm.remove has a wrong link
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dlang.org (show other issues)
Version: D2
Hardware: All All
: P2 normal
Assignee: No Owner
URL:
Keywords:
: 7195 11575 (view as issue list)
Depends on:
Blocks:
 
Reported: 2011-05-16 07:47 UTC by Andrej Mitrovic
Modified: 2014-07-19 01:18 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Andrej Mitrovic 2011-05-16 07:47:06 UTC
This: http://d-programming-language.org/phobos/std_algorithm.html#remove

points to the enum EditOp; instead of the function remove().
Comment 1 kennytm 2011-05-16 08:19:59 UTC
It's not technically wrong as the enum value's name is really 'EditOp.remove'... Perhaps those enum values shouldn't be linked.
Comment 2 Jonathan M Davis 2011-05-16 08:37:43 UTC
The way that ddoc generates links needs to be completely rewritten. It needs to be hierarchal, but it's not at all. It acts like it's just linking in a set of free functions, and I believe that it links in the first symbol with a particular name, so if you have multiple documented symbols with the same name, then it's always the first one which gets linked in, regardless of whether that's the one which would be most usefully linked. I do think that having EditOp.remove linked is of value. But the links should be arranged in such a way that their hierarchy is clear, which would mean that the link would clearly be associated with EditOp and that the remove function would have its own link. I believe that this is really a fundamental ddoc problem and not a website problem at all. And I think that the table tha std.algorithm has now is as close to a fix as you're going to get any time soon.
Comment 3 Vladimir Panteleev 2011-05-16 10:29:58 UTC
Wouldn't the problem be solved simpler by forcing it to generate unique anchor names for identifiers? I'm not sure how to understand Jonathan's comment about this, but I don't think DDoc actually thinks the two "remove"s are the same identifier.
Comment 4 Jonathan M Davis 2011-05-16 10:39:26 UTC
Ddoc always generates a link to the first identifier with a given name. How it does that, I don't know. But it makes no attempt to make the links represent any kind of hierarchy. They're purely based on the base names of identifiers and completely ignore whether a particular identifier is actually an enum value or a member of a class or struct. It's designed as if everything were a free function. The way that the links are generated is far too simplistic and needs to be redesigned. In this case, both an enum value and a function have the same name. The enum happens to be first on the page, so it gets the link.
Comment 5 Vladimir Panteleev 2011-05-16 10:40:49 UTC
My suggestion was to simply give one of them the anchor name "remove_2", or something like that.
Comment 6 Jonathan M Davis 2011-05-16 10:52:51 UTC
They do need unique anchor names, but those anchor names should probably include the hiercharchy in some manner rather than simply having numbers tagged onto them - e.g. #EditOp__remove and #remove. The core problem is that ddoc doesn't care about anything other than the base identifier name. It makes no attempt to worry about the hiercharchy or about duplicate names. And honestly, it shouldn't have to worry about duplicate names, because duplicate names are illegal in the actual code. The duplication occurs because it's effectively treating all of the identifiers as if they were at module-level.

Regardless, unique anchor names will be necessary. But I don't think that simply tacking on numbers is the right way to go about it. The fundamental problem needs to be solved. If you want to see a really bad example of the problem, looke at std.datetime. Several structs in there have the same function name, and the links become pretty useless.
Comment 7 Vladimir Panteleev 2011-05-16 10:56:42 UTC
Yeah, I never said that this is a GOOD solution, but it's a quick and dirty one, and it'll allow linking to stuff in StackOverflow answers until someone fixes it properly ;)

> Several structs in there have the same function name, and the links become pretty useless.

Hmm, so what should be done about this? Serialize the function signature into the anchor name?
Comment 8 Jonathan M Davis 2011-05-16 11:11:48 UTC
I don't think that we should be linking to every function overload, so the function signature isn't really the issue. It's the hierarchy. So, that needs to end up in the link somehow. So, for instance, if you took the year property that Date, DateTime, and SysTime have in std.datetime, you'd need something like #Date__year, #DateTime__year, and #SysTime__year. It would have to understand the hierarchy and concatenate the pieces of the hiearchy in some manner. __ makes some sense since it's unlikely to ever be the case that you'll end up with underscores at the beginning and/or end of identifiers such that the resulting anchor would conflict with the anchor of an identifier, but exactly what would be picked isn't necessarily all that important. It just needs to be unlikely to result in anchor names which conflict with other anchor names.
Comment 9 Vladimir Panteleev 2011-05-16 11:17:40 UTC
> __ makes some sense since it's unlikely to ever be the case that you'll
> end up with underscores at the beginning and/or end of identifiers such that

You can put periods in anchor names (MediaWiki uses them to escape other characters, for example), so there's no need for underscores.
Comment 10 Jonathan M Davis 2011-05-16 11:24:05 UTC
Ah. I didn't know that. I wrongly assumed that it wouldn't. That solves the problem quite nicely then. The obvious solution is then to represent the hiearchy directly. So, remove gets the anchor #EditOp.remove.
Comment 11 Jonathan M Davis 2012-01-01 15:36:08 UTC
*** Issue 7195 has been marked as a duplicate of this issue. ***
Comment 12 Brad Anderson 2012-03-11 15:15:42 UTC
Adam D. Ruppe has a pull request that includes a fix for this.

https://github.com/D-Programming-Language/dmd/pull/770
Comment 13 Andrej Mitrovic 2014-04-23 13:13:17 UTC
This seems to be fixed locally thanks to https://github.com/D-Programming-Language/dmd/pull/1174 and the updated 'MYREF' macro in std.algorithm, but the changes are not yet synced with the server, it currently points to:

http://dlang.org/phobos/std_algorithm.html#remove

Instead of:
http://dlang.org/phobos/std_algorithm.html#.remove
Comment 14 Brad Anderson 2014-06-24 19:46:32 UTC
*** Issue 11575 has been marked as a duplicate of this issue. ***
Comment 15 hsteoh 2014-06-24 20:11:04 UTC
Has the dlang.org docs been updated to document $DDOC_ANCHOR?
Comment 16 hsteoh 2014-06-24 20:52:35 UTC
Pull 1174 does *not* fix the problem, unfortunately. :-( Struct members still show up as global names rather than qualified with the containing struct name.
Comment 17 hsteoh 2014-06-24 21:15:46 UTC
Correction: pull 1174 fixes the problem for non-template members, but members of template structs, etc., still show up as top-level symbols.
Comment 18 hsteoh 2014-06-24 22:29:44 UTC
I've found the reason: emitAnchorName() is unable to find the parent symbol of a template member because the .parent pointer is only set when semantic is run, but at the time of ddoc generation, the template may not even be instantiated yet, so all the .parent pointers of the member DSymbol's are NULL.
Comment 19 hsteoh 2014-06-25 00:32:59 UTC
Found the fix: https://github.com/D-Programming-Language/dmd/pull/3693

Unfortunately, it seems to be unrelated to the specific issue in this bug; see instead: issue #10366.
Comment 20 hsteoh 2014-07-19 01:18:53 UTC
Fixed in git HEAD (as currently shown on dlang.org under the Phobos prerelease section).