Skip to content

Restructure the POD of macros #1244

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: PG-2.20
Choose a base branch
from
Open

Restructure the POD of macros #1244

wants to merge 4 commits into from

Conversation

pstaabp
Copy link
Member

@pstaabp pstaabp commented Jun 9, 2025 •
edited
Loading

For consistency the POD of macros have been updated so that there is a =head1 NAME with the following structure:

macroName.pl - short description.

Also, any method or function names will have the form:

=head2 function_name

This is helpful in conjunction with openwebwork/webwork2#2733 to extract information from the macro files.

@pstaabp pstaabp force-pushed the restructure-pod-macros branch 3 times, most recently from ea9d812 to 52d9c92 Compare June 10, 2025 11:29
@pstaabp pstaabp mentioned this pull request Jun 10, 2025
Closed
@pstaabp pstaabp force-pushed the restructure-pod-macros branch 3 times, most recently from 21d6afc to daac4b1 Compare June 17, 2025 18:24
Alex-Jordan
Alex-Jordan approved these changes Jun 18, 2025
View reviewed changes
Copy link
Contributor

@Alex-Jordan Alex-Jordan left a comment

Choose a reason for hiding this comment

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

I'm trusting that this is only POD edits as advertised. I'm not actually reading over all the changes.

@pstaabp pstaabp force-pushed the restructure-pod-macros branch from daac4b1 to 39ce4ca Compare June 21, 2025 02:36
drgrice1
drgrice1 requested changes Jun 24, 2025
View reviewed changes
Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

There are several changes that are needed.

Make sure that if a file has =encoding utf8 that that is NOT removed. Also, note that there is also a unicode character used in macros/parsers/parserLinearInequality.pl, and so that file should have =encoding utf8 added to the beginning. Please add that.

Go through all files that you added comments at the beginning regarding deprecation and such, and remove those comments. We could start a GitHub discussion about that, but it should not be added to the code.

macros/contexts/contextBoolean.pl Outdated
@@ -13,7 +17,7 @@ =head1 DESCRIPTION

Context('Boolean');

=head2 CONSTANTS
=head3 CONSTANTS
Copy link
Member

Choose a reason for hiding this comment

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

This jumps from head1 to head3 and skips head2 which should never be done. podchecker gives several warnings like *** WARNING: =head3 without preceding higher level at line 20 in file macros/contexts/contextBoolean.pl because of this. This also causes odd display issues in the sidebar of the generated HTML for the POD. Drop the =head3s to =head2s.

Copy link
Member Author

Choose a reason for hiding this comment

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

I may switch this to =head1, since I'm using the values of =head2 to performing searches on.

Copy link
Member

Choose a reason for hiding this comment

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

That is no longer the case in openwebwork/webwork2#2759. That was a bad idea as I mentioned in that pull request. The problem is that there is no guarantee that methods or functions will be at the header level of 2, and the Pod::Simple::SimpleTree does not provide the structure to divine the level that methods or functions will be at. So instead my approach just uses "terms" from the POD headers in general. It disregards words like "synopsis" and "description" and such.

macros/ui/source.pl Outdated
@@ -1,3 +1,5 @@
# Note: this probably no longer works using CGI unless specifically set up. Deprecate?
Copy link
Member

Choose a reason for hiding this comment

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

As with all of these comments that you have added at the beginnings of files, remove it. Also, note that this should not be deprecated. It does take special set up (and always has), but it still has its uses and does still work. Of course it should never be used in a problem for a course, and should not be used in problems added to the OPL.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking depcrecating this because I didn't think that CGI scripts would currently work with most WW installations--maybe reconfigure mojolicious to do this for us.

I added comments at the top of other lines--I meant to ask others about deprecating other macros.

Copy link
Member

Choose a reason for hiding this comment

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

It only works when serving webwork2 via a proxy (like apache2 or nginx). That doesn't mean it should be deprecated. This still has its uses. The point of this is for rather special cases, like wanted a convenient button to show the problem source when you are giving a problem authoring presentation. As I noted, this should not be used for production problems that will be used in a course. That has always been the case.

drgrice1
drgrice1 reviewed Jun 26, 2025
View reviewed changes
pstaabp added 3 commits June 30, 2025 17:26
@pstaabp
Restructure the POD of macros
4e7b821 
For consistency the POD of macros have been updated so that there is a `=HEAD1 NAME` with the following structure:

```
macroName.pl - short description.
```
@pstaabp
Renamed header "MACROS" to "FUNCTIONS"
dbd2785
@pstaabp
Fixes a number of POD warnings/errors
b7c2dae
@pstaabp pstaabp force-pushed the restructure-pod-macros branch from 48fe516 to b7c2dae Compare June 30, 2025 21:49
@drgrice1
Copy link
Member

By the way, one thing I noticed while looking at this pull request is that there is a very common misspelling in the POD of PG. The word SYNOPSIS is often spelled SYNPOSIS. The following files have this mispelled:

  • lib/AnswerHash.pm
  • lib/AnswerIO.pm
  • lib/Applet.pm
  • lib/Circle.pm
  • lib/Fun.pm
  • lib/Hermite.pm
  • lib/Label.pm
  • lib/Parser/Legacy/PGcomplexmacros.pl
  • lib/VectorField.pm
  • lib/WWPlot.pm
  • lib/WeBWorK/PG/EquationCache.pm
  • lib/WeBWorK/PG/ImageGenerator.pm
  • lib/WeBWorK/PG/RestrictedClosureClass.pm
  • lib/WeBWorK/PG/Translator.pm
  • macros/PG.pl
  • macros/answers/extraAnswerEvaluators.pl
  • macros/core/PGanswermacros.pl
  • macros/core/PGessaymacros.pl
  • macros/deprecated/CofIdaho_macros.pl
  • macros/deprecated/PGsequentialmacros.pl
  • macros/graph/PGgraphmacros.pl
  • macros/graph/PGstatisticsGraphMacros.pl
  • macros/math/LinearProgramming.pl

It wouldn't hurt to fix that.

@pstaabp
Fixes more POD errors/warnings in modules
0cd1971 
Also fixes spelling of SYNOPSIS
@pstaabp
Copy link
Member Author

pstaabp commented Jul 1, 2025

Changed the spelling of SYNOPSIS and fixed a number of other POD errors in modules seen using podchecker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drgrice1 drgrice1 drgrice1 requested changes

@Alex-Jordan Alex-Jordan Alex-Jordan approved these changes

@somiaj somiaj somiaj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pstaabp @drgrice1 @Alex-Jordan @somiaj