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

Fix invalid L<...> usage in POD. #864

Merged
merged 1 commit into from
Jun 27, 2023
Merged

Conversation

drgrice1
Copy link
Member

@drgrice1 drgrice1 commented Jun 25, 2023

With this change the current pod parsing script will still work. Without this change the updated pod parsing script in openwebwork/webwork2#2083 will not correctly resolve these links, because the incorrect parsing that is currently being done for these invalid links is removed.

The most prevalent invalid L<...> usage comes in the form of L<macros/macroName.pl>. Following the correct usage rules this means a link to the header "macroName.pl" in the "macros" package. Clearly that is not correct.

See https://perldoc.perl.org/perlpod#L-a-hyperlink for the L<...> usage rules. The updated script will honor those rules with the additional rules for local files that fit into that scheme:

  • For a link to a PG module, use the package name (exactly as the rules for L<...> dictate).
  • For a link to a .pod file, just use the basename without the .pod extension. So for example for a link to doc/MathObjects/UsingMathObjects.pod, use L. (See below)
  • For a link to a macro file (or script), use the basename of the macro file with the .pl extension and no path. This is actually how it already is in most files, except those fixed here.

This also fixes the L<MathObjects> links. Those don't currently work, and won't start to work with the updated script. There is no MathObjects module or pod file. (There is a MathObjects.pl file, but that doesn't have good POD.) So this changes those to go to the doc/MathObjects/UsingMathObjects.pod file.

With this change the current pod parsing script will still work.
Without this change the updated pod parsing script that is coming will
not work, because the incorrect parsing that is currently being done for
this invalid links is going to be removed.

The most prevalent invalid `L<...>` usage comes in the form of
`L<macros/macroName.pl>`.  Following the correct usage rules this means
a link to the header "macroName.pl" in the "macros" package.  Clearly
that is not correct.

See https://perldoc.perl.org/perlpod#L-a-hyperlink for the `L<...>`
usage rules.  The updated script will honor those rules with the
additional rules for local files that fit into that scheme:

* For a link to a PG module, use the package name (exactly as the rules
  for `L<...>` dictate).
* For a link to a `.pod` file, just use the basename without the `.pod`
  extension.  So for example for a link to
  doc/MathObjects/UsingMathObjects.pod, use L<UsingMathObjects>.  (See
  below)
* For a link to a macro file (or script), use the basename of the macro
  file with the `.pl` extension and no path.  This is actually how it
  already is in most files, except those fixed here.

This also fixes the `L<MathObjects>` links.  Those don't currently work,
and won't start to work with the updated script.  There is no
`MathObjects` module or pod file.  (There is a MathObjects.pl file, but
that doesn't have good POD.) So this changes those to go to the
`doc/MathObjects/UsingMathObjects.pod` file.
Copy link
Member

@pstaabp pstaabp 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.


L<macros/quickMatrixEntry.pl>
quickMatrixEntry.pl
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this one supposed to have the L<...>?

Copy link
Member Author

Choose a reason for hiding this comment

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

That macro does not have POD. So there is nothing to link to in the generated POD files. So no.

@pstaabp pstaabp merged commit 4029b96 into openwebwork:PG-2.18 Jun 27, 2023
@drgrice1
Copy link
Member Author

Oops. I was wrong in my statement that this will work with the existing script. The macro links still work, but the module links don't. Open the link http://your.server.address/webwork2/pod/lib/Value/Matrix.pm, and you will see that the MatrixReal1 and Matrix links don't work, and that the macro links do. Oh well, there are only those two and one more in tableau.pl, and they will be fixed once openwebwork/webwork2#2083 is merged.

@drgrice1 drgrice1 deleted the pod-link-fix branch June 29, 2023 20:55
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

Successfully merging this pull request may close these issues.

3 participants