Skip to content

Show Deprecated macros warning in PG editor and library Browser #2752

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

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

pstaabp
Copy link
Member

@pstaabp pstaabp commented Jun 23, 2025

This uses code from @somiaj to take the list of deprecated macros in the PG object and display only for instructors in the PGeditor and library browser.

Note: this needs openwebwork/pg#1256.

@pstaabp
Copy link
Member Author

pstaabp commented Jun 24, 2025

Updated to use the macroFileList within PG_loadMacros field of the PGcore object.

@drgrice1
Copy link
Member

@pstaabp: I added a pull request to this branch that fixes some things. First, it only processes the $pg->{pgcore}{PG_locaMacros}{macroFileList} if $pg->{pgcore} is actually a PGcore object. Second, it improves the appearance of the warning generated in the javascript. You added p tags, but those aren't needed and mess up the margins giving inconsistent spacing, so those were removed as well as some other minor styling added.

@pstaabp
Copy link
Member Author

pstaabp commented Jun 27, 2025

Merged @drgrice1 's PR as noted above.

Copy link
Contributor

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

One very minor thing, it seems that when chain loading macros, the chain loaded macros are listed before the originally loaded macro. For instance loadMacros('littleneck.pl') gives the following list of deprecated macros:

  • problemRandomize.pl
  • littleneck.pl

So even though the only macro loaded was littleneck.pl, a macro that was never loaded got listed first. I can't think of a way to fix this, just something I noticed.

@drgrice1
Copy link
Member

@somiaj: The order that the macros are displayed does not depend on if one macro loads the other at all. It also doesn't depend on the order that the macros are loaded at all either. The macros are stored in the macroFileList hash of the PGloadfiles object. Since it is a hash it is not ordered. So the order the macros are displayed will be completely random. One time you will see problemRandomilze.pl first and littleneck.pl second. Another time you will see the opposite order. I just tried a problem that uses the littleneck.pl macro, and saw both orders in several renderings.

@somiaj
Copy link
Contributor

somiaj commented Jun 28, 2025

Ahh, makes sense, it skipped my mind it was originally stored as a hash, which won't have order.

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.

I think this looks good at this point.

Although, if we merge this, then there are now going to be warnings in the library browser, on the set details page, on the statistics page, on the problem grader page, and in the PG problem editor. So these message will be quite prevalent for instructors, and there are thousands of problems in the OPL that will give these warnings. Notably, many problems use the AnswerFormatHelp.pl macro. So we need to start working on those problems. Particularly replacing AnswerFormatHelp.pl usage with helpLink usage.

@dlglin
Copy link
Member

dlglin commented Jul 3, 2025

Peter submitted openwebwork/webwork-open-problem-library#1279 to replace AnswerFormatHelp with helpLink, though I still don't like pushing out these warnings to all instructors. There are many problems in the OPL which use deprecated macros for which there aren't alternative problems available, and most instructors using these problems aren't in a position to update the problem code.

I think it's at least as likely that instructors see these warnings that the problems should not be used, and conclude that WeBWorK can't provide them with enough working problems.

@drgrice1
Copy link
Member

drgrice1 commented Jul 3, 2025

I agree that we may not be ready for this yet. I think that work on OPL problems to fix deprecated macro usage should come first.

@pstaabp
Copy link
Member Author

pstaabp commented Jul 7, 2025

I can update this to go to develop if desired to wait a year.

@pstaabp pstaabp changed the base branch from WeBWorK-2.20 to develop July 8, 2025 18:57
@pstaabp pstaabp force-pushed the deprecated-macros-warning branch 2 times, most recently from 7df0708 to 3eb23a6 Compare July 8, 2025 19:02
@pstaabp
Copy link
Member Author

pstaabp commented Jul 8, 2025

Note: switched the destination branch to develop.

somiaj and others added 5 commits July 8, 2025 15:04
This adds a warning next to the problem comment on the PGProblemEditor,
Library Browser (and maybe other places that use the Library Browser
render) right before any instructor problem if deprecated macros are
used in the problem.
Only parse the macro file list if the PGcore object is in the result.

Also, remove the `p` tags and tweak the style of the html generated to
display the deprecated macros list to get consistent spacing.
@pstaabp pstaabp force-pushed the deprecated-macros-warning branch from 3eb23a6 to 5351653 Compare July 8, 2025 19:04
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.

4 participants