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

id of Anchor is wrong #4

Open
yuki-kimoto opened this issue Apr 12, 2022 · 12 comments
Open

id of Anchor is wrong #4

yuki-kimoto opened this issue Apr 12, 2022 · 12 comments
Assignees

Comments

@yuki-kimoto
Copy link

From metacpan/metacpan-web#2668

Grinnz said "This can be discussed in https://metacpan.org/dist/MetaCPAN-Pod-XHTML if necessary, as this behavior is controlled by those roles."

I post this issue.

The id of anchor is wrong when there are multiple head names that start with the same string.

=head1 Method Names

Method Names.

=head1 Method

Method.

=head2 Method Definition

Method Definition.

L<"Method">

L<"Method"> want to jump to "=head1 Method", but it jumps to "=head1 Method Names".

Pod Renderer renders the following HTML.

<h1 id="Method-Names"><a href="#Method-Names" class="anchor"><span class="fa fa-bookmark black"></span></a><a id="Method"></a>Method Names</h1>

<p>Method Names.</p>

<h1 id="Method1"><a href="#Method1" class="anchor"><span class="fa fa-bookmark black"></span></a>Method</h1>

<p>Method.</p>

<h2 id="Method-Definition"><a href="#Method-Definition" class="anchor"><span class="fa fa-bookmark black"></span></a><a id="Method2"></a>Method Definition</h2>

<p>Method Definition.</p>

<p><a href="#Method">"Method"</a></p></div>

Is this behavior changed?

@haarg
Copy link
Member

haarg commented Apr 15, 2022

This is definitely wrong, but fixing it is not easy.

The ids are generated as the Pod is parsed. Extra targets are generated for the first word of the heading, as it's very common for people to include things like parameter specifications in headings but try to link with just the function or method name. Since they are generated as the Pod is parsed, the abbreviated ids can conflict with future headings that consist of only the first word of a previous heading. To avoid this, the additional ids would need to be generated after parsing the entire document and inserted.

@yuki-kimoto
Copy link
Author

yuki-kimoto commented Apr 15, 2022

I understand now the following heading create additional "foo" id .

=head1 foo ($nums)

This is definitely wrong, but fixing it is not easy.

To avoid this, the additional ids would need to be generated after parsing the entire document and inserted.

I will read the source codes of MetaCPAN-Pod-XHTML.

Could I try to fix this?

The fixing is "If there is an exact matching heading, the correct id is given to the heading"

=head1 Method Names -> Method1

Method Names.

=head1 Method -> Method

Method.

=head2 Method Definition -> Method2

Method Definition.

L<"Method">

What could be the problem with the fix?

@haarg
Copy link
Member

haarg commented Apr 18, 2022

The problem is that given the pod:

=head1 Method Names

Method Names.

=head1 Method

Method.

By the time the parser sees =head1 Method it has already generated the HTML for =head1 Method Names, which includes the Method target.

You are certainly welcome to try to fix this.

@yuki-kimoto
Copy link
Author

Thank you to allow me to trying fix this.

I will try to fix this in few weeks.

@yuki-kimoto
Copy link
Author

I try to fix this from today.

@yuki-kimoto
Copy link
Author

@haarg

I'm reading source codes.

I want to get original POD that is passed to "parse_string_document" to get heading information.

https://github.com/metacpan/MetaCPAN-Pod-XHTML/blob/master/t/accurate-targets.t#L33

but Pod::Simple doesn't save original POD.

https://metacpan.org/module/Pod::Simple/source#L418

I want to hook "parse_string_document" to get heading information.

How do you think this?

@haarg
Copy link
Member

haarg commented Jun 3, 2022

I made an initial implementation of this in #5, but I'm going to hold off on releasing it for now.

I've worked on getting some fixes and refactoring merged into Pod::Simple itself. Once there is a new release of that, I'll be updating this dist to work with it, and then merging in something like I have in the PR I prepared for this issue.

@yuki-kimoto
Copy link
Author

Thank you very much! pre_process method seems to allow us to parse whole html before parsing.

I wait for the merge.

@haarg
Copy link
Member

haarg commented Jul 12, 2022

This should be fixed in 0.003000.

@haarg haarg closed this as completed Jul 12, 2022
@yuki-kimoto
Copy link
Author

Thank you very much!

@yuki-kimoto
Copy link
Author

This bug does not appear to be fixed in metaCPAN or Pod Renderer.

@oalders
Copy link
Member

oalders commented Apr 19, 2023

We may need to update the dep in metacpan-api

@oalders oalders reopened this Apr 19, 2023
@oalders oalders self-assigned this Apr 19, 2023
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

No branches or pull requests

3 participants