Skip to content

Fix searching built-in operators #43

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

Closed
wants to merge 2 commits into from
Closed

Conversation

ppisar
Copy link

@ppisar ppisar commented Aug 15, 2019

ppisar added 2 commits August 15, 2019 14:35
Please note that it bundles perlop POD from Perl 5.30.0 to have
a non-moving test target.

CPAN RT#86506
perlop documents many operators before "Regexp Quote-Like Operators"
(X<operator, regexp>) section. A change introduced with "Refactor
search_perlop RT#86506" (d8b23dc) commit started to ignore those
operators. E.g. A search for '==' did not found anything. A search for
'<>' returned too many text and broke POD syntax.

This patch searches for X<> index entries in all sections and
considers =head keywords in addition to =item as section delimeters.

Because some X<> entries exists on more places, this patch implements
this strategy: First =item section that contains the X<> entry is
returned. If there is no =item sections, last =head section is
returned. If the =item entry is empty (like for 'tr'), the the output
continues up to and including a next non-empty =item. This strategy is
implemented in one pass.
@briandfoy
Copy link
Owner

I've just taken over the maintenance of Pod::Perldoc, so I'll look into this. I know it's years old, and it might take me a bit to catch up.

@briandfoy briandfoy added Type: enhancement improve a feature that already exists Status: stalled something is blocking progress Priority: low get to this whenever labels Dec 5, 2023
@haarg
Copy link
Contributor

haarg commented Dec 6, 2023

I think it would be better to include only a representative fragment of perlop for testing this. Including the full document will be similar to perlfunc and cause continual confusion when this is brought into core.

@briandfoy
Copy link
Owner

I think it would be better to include only a representative fragment of perlop for testing this. Including the full document will be similar to perlfunc and cause continual confusion when this is brought into core.

Can you expound on that? Is it the filename that's the problem, or is something looking at the contents? Are there any links to previous threads?

@briandfoy briandfoy added Priority: high work on this first and removed Priority: low get to this whenever labels Dec 7, 2023
@briandfoy
Copy link
Owner

Okay, I think I know what is going on here. I thought it was all related and that confused me.

Both of these seem to work for me, so I'll merge this fix.

@briandfoy briandfoy self-requested a review December 8, 2023 23:56
Copy link
Owner

@briandfoy briandfoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@@ -15,6 +15,7 @@ WriteMakefile(

'PREREQ_PM' => {
# Are there any hard dependencies not covered here?
'blib' => '0',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No one appears to use this, so I might remove it after I merge.

@briandfoy
Copy link
Owner

I've rebased this and re-push as #70.

@briandfoy briandfoy closed this Dec 9, 2023
@briandfoy briandfoy added Status: accepted the fix is accepted and removed Status: stalled something is blocking progress Priority: high work on this first labels Dec 9, 2023
@briandfoy briandfoy added Status: released there is a new release with this fix and removed Status: accepted the fix is accepted labels Feb 16, 2025
@briandfoy briandfoy self-assigned this Feb 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: released there is a new release with this fix Type: enhancement improve a feature that already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants