podcheck.t: Fix multiple link targets bug #22771
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
end_head1() extends SUPER::end_head1(). So call that. It turns out that this didn't matter currently, but is bad practice.
Prior to this commit, the resetting of certain variables was done just for =head1. But there is nothing special about this level for these variables, so do it for all.
podcheck looks through the source distribution for pod. It skips internal checking of ones in /cpan (unless a command line option says to. If you do this, be prepared for finding gobs of problems in these!). But any such module found is fair game to be linked to by modules we do examine. It is a simple matter to add them to the list of known modules that can be linked to. Prior to this commit this was done surreptitiously by extending the Pod::Checker node() method. But a future commit will take this away.
podcheck.t would wrongly warn about duplicate link targets. It is ambiguous if you have, say =head1 foo ... =item foo and then try to create a link to 'foo'. Which 'foo' should it go to? So podcheck looks for such ambiguous links. However, it was incorrectly warning on =head1 foo ... =item * foo The latter is a bullet list, which is not an appropriate link target. The logic was flawed in the node() method, when it encountered the second foo, it didn't realize it was for a bullet list, and pushed it onto a list which caused the count to be larger than 1, which caused it later to think there were two link targets with the same name. The solution here is to instead note that linkable targets are only in =headN or definition lists. The acquisition of the names of them are moved to the methods that get called upon the termination of each of these by the parsing routines. The count is incremented at that point. This means the node() method extension no longer does anything useful and is removed; as well as an auxiliary field, linkable_item. We no longer need to distinguish between =headN and =item.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
podcheck.t would wrongly warn about duplicate link targets. It is
ambiguous if you have, say
=head1 foo
...
=item foo
and then try to create a link to 'foo'. Which 'foo' should it go to?
So podcheck looks for such ambiguous links. However, it was incorrectly
warning on
=head1 foo
...
=item * foo
The latter is a bullet list, which is not an appropriate link target.
The logic was flawed in the node() method, when it encountered the
second foo, it didn't realize it was for a bullet list, and pushed it
onto a list which caused the count to be larger than 1, which caused it
later to think there were two link targets with the same name.
The solution here is to instead note that linkable targets are only in
=headN or definition lists. The acquisition of the names of them are
moved to the methods that get called upon the termination of each of
these by the parsing routines. The count is incremented at that point.
This means the node() method extension no longer does anything useful
and is removed; as well as an auxiliary field, linkable_item. We no
longer need to distinguish between =headN and =item.