cpan-testers / cpantesters-backend

Backend data processing for CPAN Testers
Other
0 stars 4 forks source link

Non-standard POD directives in .pm files drop content #10

Closed jkeenan closed 6 years ago

jkeenan commented 6 years ago

When I'm working with a file of Perl code (.pm .pl .pod), I expect perldoc somefile.pm to Just Work. I expect that that any non-standard directives found in pod/perlpod.pod, designed for use in custom formatters, will not cause perldoc to drop content.

The .pm files in this repository, however, use non-standard directives which drop content.

$ ack '^=(method|attr)' ./lib/CPAN/Testers
lib/CPAN/Testers/Backend/Migrate/MetabaseCache.pm
27:=attr metabase_dbh
39:=attr schema
76:=method process_sth
95:=method find_unprocessed_entries
129:=method find_entries
152:=method parse_metabase_report

lib/CPAN/Testers/Backend/Migrate/MetabaseUsers.pm
23:=attr metabase_dbh
35:=attr schema

lib/CPAN/Testers/Backend/ProcessReports.pm
48:=attr schema
60:=attr metabase_dbh
74:=method run
122:=method find_unprocessed_reports
141:=method find_reports
166:=method write_metabase_cache

This has a number of harmful effects. First, it causes the files to fail the tests in the standard podchecker utility.

 $ podchecker lib/CPAN/Testers/Backend/ProcessReports.pm
*** ERROR: Unknown directive: =attr at line 48 in file lib/CPAN/Testers/Backend/ProcessReports.pm
*** ERROR: Unknown directive: =attr at line 60 in file lib/CPAN/Testers/Backend/ProcessReports.pm
*** ERROR: Unknown directive: =method at line 74 in file lib/CPAN/Testers/Backend/ProcessReports.pm
*** ERROR: Unknown directive: =method at line 122 in file lib/CPAN/Testers/Backend/ProcessReports.pm
*** ERROR: Unknown directive: =method at line 141 in file lib/CPAN/Testers/Backend/ProcessReports.pm
*** ERROR: Unknown directive: =method at line 166 in file lib/CPAN/Testers/Backend/ProcessReports.pm
lib/CPAN/Testers/Backend/ProcessReports.pm has 6 pod syntax errors.

Second, and more importantly, it means that content is dropped from the output of perldoc, pod2text and perhaps other utilities. Here is a section from the plain-text content of lib/CPAN/Testers/Backend/ProcessReports.pm:

=attr schema

A L<CPAN::Testers::Schema> object to access the database.

=cut

has schema => (
    is => 'ro',
    isa => InstanceOf['CPAN::Testers::Schema'],
    required => 1,
);

=attr metabase_dbh

A L<DBI> object connected to the Metabase cache. This is a legacy database
needed for some parts of the web app and backend. When these parts are
updated to use the new test reports, we can remove this attribute.

=cut

Now here's what perldoc lib/CPAN/Testers/Backend/ProcessReports.pm shows in that region:

    A CPAN::Testers::Schema object to access the database.

    A DBI object connected to the Metabase cache. This is a legacy database
    needed for some parts of the web app and backend. When these parts are
    updated to use the new test reports, we can remove this attribute.

The text after the first =cut directive is not rendered.

Is there a rationale for this?

Thank you very much. Jim Keenan

preaction commented 6 years ago

I use Pod::Weaver to turn =attr into

=head1 ATTRIBUTES

=head2 <attr>

<attr text>

The rationale is mostly that it removes the need to ensure that the code is in a certain order under the given headings (which may be far away from the code being added), but also ensures I don't forget the headings (which I did, a lot). It also makes POD a bit more like the more robust systems of Javadoc and Pydoc, which just allow me to add docs to a thing and it figures out what kind of thing it is and shows the appropriate documentation.

But, yes, it does have this problem. I wrote a script that will weave the POD and display it like perldoc, but I've not yet put it on CPAN (not sure if I want to keep it named Pod::Weaver::CLI or name it App::weavedoc).

preaction commented 6 years ago

I've released App::weavedoc to the unsuspecting world (v0.001 on metacpan). This is as fixed as I can make it.