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

please consider switching to Pod::Simple from Pod::PlainText for Perl::Critic::Utils::POD #818

Open
renderorange opened this issue May 10, 2018 · 7 comments

Comments

@renderorange
Copy link

Hello! We're current using Perl::Critic::Utils::POD in our department's linter and have run into a bug with Pod::PlainText.

The issue is happening while running get_pod_section_from_file, and can be reproduced with a module containing a pod item section with nothing but verbatim text.

=item Example example

 {
   "data" : {
      "id" : "1234",
      "timestamp" : "1517232936",
      "severity" : "WARNING",
      "message" : "This is a test message"
   },
   "type" : "example"
 }

Use of uninitialized value $_ in pattern match (m//) at (...) Pod/PlainText.pm line 495, <$file_handle> line 159.

Pod::PlainText's page mentions the module and distribution should be considered legacy and recommend migrating to Pod::Simple instead. We have a fix for the Pod::PlainText bug, but since the module is considered legacy (and no activity has happened there for quite a while), thought it would be best to suggest moving to Pod::Simple here, for future maintainability.

Since Pod::Simple is an actively maintained module, please consider migrating to it. I'm happy to work on a patch to convert, if it would help.

Thanks!

@petdance
Copy link
Member

I'm in favor of this. Thanks for the offer.

Please make sure that whatever tests you create include the test case described above.

@renderorange
Copy link
Author

You're welcome. I'll start on it as soon as I have some free cycles and ensure this case is well tested.

@petdance
Copy link
Member

Thanks. I'm hoping you'll be able to address the commented-out test in t/05_utils_pod.t, too.

@petdance
Copy link
Member

I just removed an unused usage of Pod::Plaintext at 7b47a2b. Looks like there's only one module that uses it now.

@petdance
Copy link
Member

And it looks like you'll be getting rid of Pod::Select as well if you're removing Pod::Parser entirely.

@renderorange
Copy link
Author

okie dokie, I will look into both, as well as any tests needing maintenance.

@renderorange
Copy link
Author

renderorange commented May 14, 2018

Hello. After digging into the Utils/POD.pm module the switch seems fairly simple. The biggest hurdle appears to be removing Pod::Select. There is a module within the Pod::Simple family which implements select(), Pod::Simple::Select, however, the requirements for the module's installation appear to target Perl 5.24. Tie::File and Carp both aren't as high as needed for lesser Perl versions. So, I'm still trying to find a solution which will apply to a broader range of users. There's an open case on the Pod::Simple::Select bug tracker inquiring why it's module requirements are set so high.

Another change as I've been digging in has been to consider Pod::Text, which appears to be the like implementation of Pod::PlainText within the Pod::Simple family. So far I've been able to drop in Pod::Text and Pod::Simple for most of the parser locations in POD.pm and still pass all of the tests.

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

2 participants