Skip to content

Comments

Remove leading dot from DDOC_ANCHOR#1185

Closed
JakobOvrum wants to merge 1 commit intodlang:masterfrom
JakobOvrum:ddocanchor_remove_leading_dot
Closed

Remove leading dot from DDOC_ANCHOR#1185
JakobOvrum wants to merge 1 commit intodlang:masterfrom
JakobOvrum:ddocanchor_remove_leading_dot

Conversation

@JakobOvrum
Copy link
Contributor

The leading dot was introduced in #179 to avoid conflicts with legacy fragment names.

However, the legacy fragment name scheme is not compatible with HTML. Fragment names must be unique in the document, whether using name (removed in HTML5) or id, otherwise behaviour is undefined. For example, for http://dlang.org/phobos/std_stdio#writeln, Firefox links to the name="writeln" appearing earliest in the document, which is actually std.stdio.File.writeln, as opposed to the std.stdio.writeln one would expect.

With this PR, only the qualified name (rooted in the module) emits a fragment, so we can remove the leading dot.

Apropos name, it should be changed to id everywhere for HTML5 compatbility. This PR only does it in one place.

@adamdruppe
Copy link
Contributor

Looks good to me. If there's no objections by the end of the day, someone ping me and I'll hit the merge myself.

@CyberShadow
Copy link
Member

Does this fix anything in practice? Because obviously it does break legacy links, and breaking links is bad practice.

@aG0aep6G
Copy link
Contributor

aG0aep6G commented Feb 4, 2016

How about generating both dot-prefixed and non-prefixed anchors via DDOC_ANCHOR? I.e.:

DDOC_PSYMBOL = $(SPANC ddoc_psymbol, $0)
DDOC_ANCHOR = $(ADEF .$1)$(ADEF $1)$(DIVCID quickindex, quickindex.$1, )

This should get us rid of the duplicates and keep all links working.

@adamdruppe
Copy link
Contributor

I suspect those legacy links were already really broken. I never used them because they wouldn't link to the right place anyway!

@LightBender
Copy link
Contributor

PR closed as stalled. If you wish to resurrect it you are welcome to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants