Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

work around duplicate id= anchors #632

Merged
merged 1 commit into from
Aug 5, 2016
Merged

work around duplicate id= anchors #632

merged 1 commit into from
Aug 5, 2016

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Jun 24, 2016

This should address #631.

I'd rather someone else consider it (the output does change, it becomes wordier).

@AlexDaniel
Copy link
Member

It has been a while since you submitted this pull request. What do you think about it now?

@AlexDaniel
Copy link
Member

OK, so if one takes a look at #561, it is easy to notice that duplicate anchors is the last big issue.

Therefore it is probably time to do something with this.

There are basically two options:

  1. 👍 “Yes, let's fix the docs now!”
  2. 👎 “No, we should fix it in Pod::To::HTML instead”… (or maybe change this pull request slightly?)

Below you can see before/after screenshots. It does become wordier, and it does introduce a bit of redundant information (the name of the class is displayed at least twice).

Also, “Any method” sounds really weird. I don't like it at all. Especially note these real cases:

  • “Any method any”
  • “Routine method multi”
  • “Cool routine roots”
  • “Any method end”

A bit funny sometimes, but we can probably live with that. Still, I think beginners will find it hard to read (or not? Am I the only one having troubles?), can we do something about it?

I am going to merge this if nobody dares to oppose, so please speak up.

By the way, it affects all pages. And that's for a good reason! For example, take a look at IO::Handle#method_say – there are two methods “say” on that page, but the second link is broken because of duplicate ids. So yeah, it makes sense.


Before

screenshot before

After

screenshot after

@coke
Copy link
Collaborator

coke commented Aug 5, 2016

Perhaps instead of

Any method eager

use

(Any) method eager

@Altai-man
Copy link
Member

Altai-man commented Aug 5, 2016

The workaround is really small. When we'll fix our Html::To::Pod, it'll be super-easy to revert this back. So, generally, I'm +1 about this. But it is not a final solution.

We still have dead links at the routine/ pages, it's my fault(or maybe not, since this part was completely broken, but just a little bit broken now because of me). I'll prepare a complete patch quickly(till today evening), so please, wait until it.
Patch are pushed, so I have nothing against merging now. But brackets, as @coke said, will be pretty.

@AlexDaniel AlexDaniel merged commit bf349d8 into Raku:master Aug 5, 2016
@AlexDaniel AlexDaniel mentioned this pull request Aug 5, 2016
Altai-man added a commit that referenced this pull request Aug 6, 2016
@jsoref jsoref deleted the dup-id branch March 8, 2017 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants