From 1f5664c1b692f09fc42a313a2366430e6a59f79c Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Fri, 20 Jun 2025 12:09:43 -0700 Subject: [PATCH 1/3] Update the pgeditor to use the pgcritic to analyze a problem. --- htdocs/js/PGProblemEditor/pgproblemeditor.js | 26 ++++ lib/WebworkWebservice.pm | 1 + lib/WebworkWebservice/ProblemActions.pm | 22 ++- .../PGProblemEditor/format_code_form.html.ep | 14 ++ .../PGProblemEditor/pg_critic.html.ep | 147 ++++++++++++++++++ 5 files changed, 208 insertions(+), 2 deletions(-) create mode 100644 templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep diff --git a/htdocs/js/PGProblemEditor/pgproblemeditor.js b/htdocs/js/PGProblemEditor/pgproblemeditor.js index 286840cc16..2599d36011 100644 --- a/htdocs/js/PGProblemEditor/pgproblemeditor.js +++ b/htdocs/js/PGProblemEditor/pgproblemeditor.js @@ -235,6 +235,30 @@ .catch((err) => showMessage(`Error: ${err?.message ?? err}`)); }; + // Send a request to the server to run the PG critic in the CodeMirror editor. + const runPGCritic = () => { + const request_object = { courseID: document.getElementsByName('courseID')[0]?.value }; + + const user = document.getElementsByName('user')[0]; + if (user) request_object.user = user.value; + const sessionKey = document.getElementsByName('key')[0]; + if (sessionKey) request_object.key = sessionKey.value; + + request_object.rpc_command = 'runPGCritic'; + request_object.pgCode = + webworkConfig?.pgCodeMirror?.source ?? document.getElementById('problemContents')?.value ?? ''; + + fetch(webserviceURL, { method: 'post', mode: 'same-origin', body: new URLSearchParams(request_object) }) + .then((response) => response.json()) + .then((data) => { + renderArea.innerHTML = data.result_data.html; + }) + .catch((err) => { + console.log(err); + showMessage(`Error: ${err?.message ?? err}`); + }); + }; + document.getElementById('take_action')?.addEventListener('click', async (e) => { if (document.getElementById('current_action')?.value === 'format_code') { e.preventDefault(); @@ -244,6 +268,8 @@ document.querySelector('input[name="action.format_code"]:checked').value == 'convertCodeToPGML' ) { convertCodeToPGML(); + } else if (document.querySelector('input[name="action.format_code"]:checked').value == 'runPGCritic') { + runPGCritic(); } return; } diff --git a/lib/WebworkWebservice.pm b/lib/WebworkWebservice.pm index d8f9a28a5e..3393cc3b14 100644 --- a/lib/WebworkWebservice.pm +++ b/lib/WebworkWebservice.pm @@ -255,6 +255,7 @@ sub command_permission { putPastAnswer => 'problem_grader', tidyPGCode => 'access_instructor_tools', convertCodeToPGML => 'access_instructor_tools', + runPGCritic => 'access_instructor_tools', # WebworkWebservice::RenderProblem renderProblem => 'proctor_quiz_login', diff --git a/lib/WebworkWebservice/ProblemActions.pm b/lib/WebworkWebservice/ProblemActions.pm index 3252bcdaa8..dbb728aeff 100644 --- a/lib/WebworkWebservice/ProblemActions.pm +++ b/lib/WebworkWebservice/ProblemActions.pm @@ -6,8 +6,9 @@ use warnings; use Data::Structure::Util qw(unbless); -use WeBWorK::PG::Tidy qw(pgtidy); -use WeBWorK::PG::ConvertToPGML qw(convertToPGML); +use WeBWorK::PG::Tidy qw(pgtidy); +use WeBWorK::PG::ConvertToPGML qw(convertToPGML); +use WeBWorK::PG::PGProblemCritic qw(analyzePGcode); sub getUserProblem { my ($invocant, $self, $params) = @_; @@ -165,4 +166,21 @@ sub convertCodeToPGML { } +sub runPGCritic { + my ($invocant, $self, $params) = @_; + my $pg_critic_results = analyzePGcode($params->{pgCode}); + + my $html_output = $self->c->render_to_string( + template => 'ContentGenerator/Instructor/PGProblemEditor/pg_critic', + results => $pg_critic_results + ); + + return { + ra_out => { + html => $html_output + }, + text => 'The script pg-critic has been run successfully.' + }; +} + 1; diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/format_code_form.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/format_code_form.html.ep index 69da3d5a95..0ddab6d09f 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor/format_code_form.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/format_code_form.html.ep @@ -30,4 +30,18 @@ +
+ <%= radio_button 'action.format_code' => 'runPGCritic', + id => 'action_format_code_run_pgcritic', class => 'form-check-input'=%> + <%= label_for 'action_format_code_run_pgcritic', class => 'form-check-label', begin =%> + <%== maketext('Run the PG Critic Analyzer') =%> + <% end =%> + + + + +
diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep new file mode 100644 index 0000000000..d5a4e77f1d --- /dev/null +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep @@ -0,0 +1,147 @@ +
+

PG Critic Results

+ +

Metadata

+ +

The following lists required metadata. If any is missing, the given tag must be filled in. + However, make sure that the categories are correct, especially if the problem has been + copied.

+ +% sub showIcon { my $show = shift; +% return $show ? q! +% +% +% ! : q! +% +% +% !; +%} + + + + + + + +
DBsubject <%== showIcon($results->{metadata}{DBsection}) %>
DBchapter <%== showIcon($results->{metadata}{DBchapter}) %>
DBsection <%== showIcon($results->{metadata}{DBsection}) %>
Keywords <%== showIcon($results->{metadata}{KEYWORDS}) %>
+ +% my $pos = $results->{positive}; +% if ($pos->{PGML} || $pos->{solution} || $pos->{hint}) { +

Good aspects of this problems are the following

+% } + + +% if ($pos->{PGML}) { + +% } +% if ($pos->{solution}) { + +% } +% if ($pos->{hint}) { + +% } +% # list of the positive contexts: +% my @good_contexts = grep { $pos->{contexts}{$_} } keys %{$pos->{parsers}}; +% if (@good_contexts) { + +% } +% my @good_parsers = grep { $pos->{parsers}->{$_} } keys %{$pos->{parsers}}; +% if (@good_parsers) { + +% } +% my @good_macros = grep { $pos->{macros}->{$_} } keys %{$pos->{macros}}; +% if (@good_macros) { + +% } + +
PGMLThis problem uses PGML, the current preferred way to write problem (text), solution and hint + blocks.
SolutionsThis problem has a solution block. Every problem should have solutions that the + student can view after the answer data.
HintsThis problem has a hint. This can be helpful for students after attempting the problem + a few times (this can be set by the instructor). +% } +% if ($pos->{randomness}) { +
RandomnessThis problem uses randomness. This is desired to give to a class of students, each + of whom may have a different problem.
Modern ContextsThis problem uses the following modern contexts: + <%= join(', ', @good_contexts) %>
Modern ParsersThis problem uses features of the following modern parsers: + <%= join(', ', @good_parsers) %>
Modern MacrosThis problem uses functionality from the following modern macros: + <%= join(', ', @good_macros) %>
+ + +% if( scalar(@{$results->{deprecated_macros}}) > 0) { +

Deprecated Macros

+

This problem has the following deprecated macros: <%= join(', ',@{$results->{deprecated_macros}} ) %>

+ +

These should be removed from the problem in that these macros will be deleted from PG in a future + version. The functions from these macros may be listed below to help aid in transitioning away from + these macros.

+% } + +% my $has_bad_features = 0; +% $has_bad_features += $results->{negative}{$_} for (keys %{$results->{negative}}); + +% if ($has_bad_features || !$pos->{solution}) { +

You can improve on the following:

+

There are features in this problem that contain old or deprecated features. The following + list gives feedback of how the problem can be improved.

+%} + + + + +
From d7f05844c6e375f02567dbc3a2071619f5eacd0f Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 9 Jul 2025 07:59:40 -0500 Subject: [PATCH 2/3] Add PG critic to the PG problem editor. This adds the PG critic results from `WeBWorK::PG::Critic` to the PG problem editor. Note that the "Format Code" tab in the editor is now the "Code Maintenance" tab. This is because this new option is not a code formatter. All of the options fit into the category of code maintenance, so this is a better name. --- bin/check_modules.pl | 1 + htdocs/js/PGProblemEditor/pgproblemeditor.js | 34 +-- .../Instructor/PGProblemEditor.pm | 22 +- lib/WebworkWebservice/ProblemActions.pm | 17 +- ....html.ep => code_maintenance_form.html.ep} | 26 +-- .../PGProblemEditor/pg_critic.html.ep | 220 ++++++------------ .../InstructorPGProblemEditor.html.ep | 21 +- 7 files changed, 138 insertions(+), 203 deletions(-) rename templates/ContentGenerator/Instructor/PGProblemEditor/{format_code_form.html.ep => code_maintenance_form.html.ep} (61%) diff --git a/bin/check_modules.pl b/bin/check_modules.pl index 3b73cee2bf..264b4af686 100755 --- a/bin/check_modules.pl +++ b/bin/check_modules.pl @@ -88,6 +88,7 @@ =head1 DESCRIPTION Net::OAuth Opcode Pandoc + Perl::Critic Perl::Tidy PHP::Serialization Pod::Simple::Search diff --git a/htdocs/js/PGProblemEditor/pgproblemeditor.js b/htdocs/js/PGProblemEditor/pgproblemeditor.js index 2599d36011..9ef2cd8427 100644 --- a/htdocs/js/PGProblemEditor/pgproblemeditor.js +++ b/htdocs/js/PGProblemEditor/pgproblemeditor.js @@ -161,6 +161,15 @@ ?.addEventListener('change', () => (deleteBackupCheck.checked = true)); } + const renderArea = document.getElementById('pgedit-render-area'); + + const scrollToRenderArea = () => { + // Scroll to the top of the render window if the current scroll position is below that. + const renderAreaRect = renderArea.getBoundingClientRect(); + const topBarHeight = document.querySelector('.webwork-logo')?.getBoundingClientRect().height ?? 0; + if (renderAreaRect.top < topBarHeight) window.scrollBy(0, renderAreaRect.top - topBarHeight); + }; + // Send a request to the server to perltidy the current PG code in the CodeMirror editor. const tidyPGCode = () => { const request_object = { courseID: document.getElementsByName('courseID')[0]?.value }; @@ -251,24 +260,26 @@ fetch(webserviceURL, { method: 'post', mode: 'same-origin', body: new URLSearchParams(request_object) }) .then((response) => response.json()) .then((data) => { + if (data.error) throw new Error(data.error); + if (!data.result_data) throw new Error('An invalid response was received.'); renderArea.innerHTML = data.result_data.html; + scrollToRenderArea(); }) - .catch((err) => { - console.log(err); - showMessage(`Error: ${err?.message ?? err}`); - }); + .catch((err) => showMessage(`Error: ${err?.message ?? err}`)); }; document.getElementById('take_action')?.addEventListener('click', async (e) => { - if (document.getElementById('current_action')?.value === 'format_code') { + if (document.getElementById('current_action')?.value === 'code_maintenance') { e.preventDefault(); - if (document.querySelector('input[name="action.format_code"]:checked').value == 'tidyPGCode') { + if (document.querySelector('input[name="action.code_maintenance"]:checked').value === 'tidyPGCode') { tidyPGCode(); } else if ( - document.querySelector('input[name="action.format_code"]:checked').value == 'convertCodeToPGML' + document.querySelector('input[name="action.code_maintenance"]:checked').value === 'convertCodeToPGML' ) { convertCodeToPGML(); - } else if (document.querySelector('input[name="action.format_code"]:checked').value == 'runPGCritic') { + } else if ( + document.querySelector('input[name="action.code_maintenance"]:checked').value === 'runPGCritic' + ) { runPGCritic(); } return; @@ -332,7 +343,6 @@ }); const renderURL = `${webworkConfig?.webwork_url ?? '/webwork2'}/render_rpc`; - const renderArea = document.getElementById('pgedit-render-area'); const fileType = document.getElementsByName('file_type')[0]?.value; // This is either the div containing the CodeMirror editor or the problemContents textarea in the case that @@ -416,11 +426,7 @@ } adjustIFrameHeight(); - - // Scroll to the top of the render window if the current scroll position is below that. - const renderAreaRect = renderArea.getBoundingClientRect(); - const topBarHeight = document.querySelector('.webwork-logo')?.getBoundingClientRect().height ?? 0; - if (renderAreaRect.top < topBarHeight) window.scrollBy(0, renderAreaRect.top - topBarHeight); + scrollToRenderArea(); }); const render = () => diff --git a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm index c7736812de..c668a903e1 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm @@ -90,7 +90,7 @@ the submit button pressed (the action). Requested actions and aliases View/Reload action = view Generate Hardcopy: action = hardcopy - Format Code: action = format_code + Code Maintenance: action = code_maintenance Save: action = save Save as: action = save_as Append: action = add_problem @@ -118,15 +118,15 @@ use SampleProblemParser qw(getSampleProblemCode generateMetadata); use constant DEFAULT_SEED => 123456; # Editor tabs -use constant ACTION_FORMS => [qw(view hardcopy format_code save save_as add_problem revert)]; +use constant ACTION_FORMS => [qw(view hardcopy code_maintenance save save_as add_problem revert)]; use constant ACTION_FORM_TITLES => { - view => x('View/Reload'), - hardcopy => x('Generate Hardcopy'), - format_code => x('Format Code'), - save => x('Save'), - save_as => x('Save As'), - add_problem => x('Append'), - revert => x('Revert'), + view => x('View/Reload'), + hardcopy => x('Generate Hardcopy'), + code_maintenance => x('Code Maintenance'), + save => x('Save'), + save_as => x('Save As'), + add_problem => x('Append'), + revert => x('Revert'), }; my $BLANKPROBLEM = 'newProblem.pg'; @@ -847,9 +847,9 @@ sub view_handler ($c) { return; } -# The format_code action is handled by javascript. This is provided just in case +# The code_maintenance action is handled by javascript. This is provided just in case # something goes wrong and the handler is called. -sub format_code_handler { } +sub code_maintenance_handler { } sub hardcopy_handler ($c) { # Redirect to problem editor page. diff --git a/lib/WebworkWebservice/ProblemActions.pm b/lib/WebworkWebservice/ProblemActions.pm index dbb728aeff..f308e78beb 100644 --- a/lib/WebworkWebservice/ProblemActions.pm +++ b/lib/WebworkWebservice/ProblemActions.pm @@ -6,9 +6,9 @@ use warnings; use Data::Structure::Util qw(unbless); -use WeBWorK::PG::Tidy qw(pgtidy); -use WeBWorK::PG::ConvertToPGML qw(convertToPGML); -use WeBWorK::PG::PGProblemCritic qw(analyzePGcode); +use WeBWorK::PG::Tidy qw(pgtidy); +use WeBWorK::PG::ConvertToPGML qw(convertToPGML); +use WeBWorK::PG::Critic qw(critiquePGCode); sub getUserProblem { my ($invocant, $self, $params) = @_; @@ -168,16 +168,13 @@ sub convertCodeToPGML { sub runPGCritic { my ($invocant, $self, $params) = @_; - my $pg_critic_results = analyzePGcode($params->{pgCode}); - - my $html_output = $self->c->render_to_string( - template => 'ContentGenerator/Instructor/PGProblemEditor/pg_critic', - results => $pg_critic_results - ); return { ra_out => { - html => $html_output + html => $self->c->render_to_string( + template => 'ContentGenerator/Instructor/PGProblemEditor/pg_critic', + results => [ critiquePGCode($params->{pgCode}) ] + ) }, text => 'The script pg-critic has been run successfully.' }; diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/format_code_form.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/code_maintenance_form.html.ep similarity index 61% rename from templates/ContentGenerator/Instructor/PGProblemEditor/format_code_form.html.ep rename to templates/ContentGenerator/Instructor/PGProblemEditor/code_maintenance_form.html.ep index 0ddab6d09f..a083a8edfe 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor/format_code_form.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/code_maintenance_form.html.ep @@ -1,9 +1,9 @@ % last unless $c->{is_pg};
- <%= radio_button 'action.format_code' => 'tidyPGCode', - id => 'action_format_code_perltidy', class => 'form-check-input', checked => undef =%> - <%= label_for 'action_format_code_perltidy', class => 'form-check-label', begin =%> + <%= radio_button 'action.code_maintenance' => 'tidyPGCode', + id => 'action_code_maintenance_perltidy', class => 'form-check-input', checked => undef =%> + <%= label_for 'action_code_maintenance_perltidy', class => 'form-check-label', begin =%> <%== maketext('Reformat the code using perltidy.') =%> <% end =%> - <%= radio_button 'action.format_code' => 'convertCodeToPGML', - id => 'action_format_code_convert_PGML', class => 'form-check-input'=%> - <%= label_for 'action_format_code_convert_PGML', class => 'form-check-label', begin =%> + <%= radio_button 'action.code_maintenance' => 'convertCodeToPGML', + id => 'action_code_maintenance_convert_PGML', class => 'form-check-input'=%> + <%= label_for 'action_code_maintenance_convert_PGML', class => 'form-check-label', begin =%> <%== maketext('Convert the code to PGML') =%> <% end =%> + data-bs-placement="top" data-bs-toggle="popover" role="button">
- <%= radio_button 'action.format_code' => 'runPGCritic', - id => 'action_format_code_run_pgcritic', class => 'form-check-input'=%> - <%= label_for 'action_format_code_run_pgcritic', class => 'form-check-label', begin =%> + <%= radio_button 'action.code_maintenance' => 'runPGCritic', + id => 'action_code_maintenance_run_pgcritic', class => 'form-check-input'=%> + <%= label_for 'action_code_maintenance_run_pgcritic', class => 'form-check-label', begin =%> <%== maketext('Run the PG Critic Analyzer') =%> <% end =%> + . 'code analyzer which gives suggestions on using modern PG constructs and ' + . 'ensures that you include important features.') =%>" + data-bs-placement="top" data-bs-toggle="popover" role="button"> diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep index d5a4e77f1d..3d6d1733de 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep @@ -1,147 +1,75 @@ -
-

PG Critic Results

- -

Metadata

- -

The following lists required metadata. If any is missing, the given tag must be filled in. - However, make sure that the categories are correct, especially if the problem has been - copied.

- -% sub showIcon { my $show = shift; -% return $show ? q! -% -% -% ! : q! -% -% -% !; -%} - - - - - - - -
DBsubject <%== showIcon($results->{metadata}{DBsection}) %>
DBchapter <%== showIcon($results->{metadata}{DBchapter}) %>
DBsection <%== showIcon($results->{metadata}{DBsection}) %>
Keywords <%== showIcon($results->{metadata}{KEYWORDS}) %>
- -% my $pos = $results->{positive}; -% if ($pos->{PGML} || $pos->{solution} || $pos->{hint}) { -

Good aspects of this problems are the following

-% } - - -% if ($pos->{PGML}) { - -% } -% if ($pos->{solution}) { - -% } -% if ($pos->{hint}) { - -% } -% # list of the positive contexts: -% my @good_contexts = grep { $pos->{contexts}{$_} } keys %{$pos->{parsers}}; -% if (@good_contexts) { - -% } -% my @good_parsers = grep { $pos->{parsers}->{$_} } keys %{$pos->{parsers}}; -% if (@good_parsers) { - -% } -% my @good_macros = grep { $pos->{macros}->{$_} } keys %{$pos->{macros}}; -% if (@good_macros) { - -% } - -
PGMLThis problem uses PGML, the current preferred way to write problem (text), solution and hint - blocks.
SolutionsThis problem has a solution block. Every problem should have solutions that the - student can view after the answer data.
HintsThis problem has a hint. This can be helpful for students after attempting the problem - a few times (this can be set by the instructor). -% } -% if ($pos->{randomness}) { -
RandomnessThis problem uses randomness. This is desired to give to a class of students, each - of whom may have a different problem.
Modern ContextsThis problem uses the following modern contexts: - <%= join(', ', @good_contexts) %>
Modern ParsersThis problem uses features of the following modern parsers: - <%= join(', ', @good_parsers) %>
Modern MacrosThis problem uses functionality from the following modern macros: - <%= join(', ', @good_macros) %>
- - -% if( scalar(@{$results->{deprecated_macros}}) > 0) { -

Deprecated Macros

-

This problem has the following deprecated macros: <%= join(', ',@{$results->{deprecated_macros}} ) %>

- -

These should be removed from the problem in that these macros will be deleted from PG in a future - version. The functions from these macros may be listed below to help aid in transitioning away from - these macros.

-% } - -% my $has_bad_features = 0; -% $has_bad_features += $results->{negative}{$_} for (keys %{$results->{negative}}); - -% if ($has_bad_features || !$pos->{solution}) { -

You can improve on the following:

-

There are features in this problem that contain old or deprecated features. The following - list gives feedback of how the problem can be improved.

-%} - -
    -% if ($results->{negative}{BEGIN_TEXT}) { -
  • This problem contains older formatting blocks like BEGIN_TEXT. Consider use PGML. - In the Format Code section of the PG Editor, the "Convert to PGML" should be used - as a start to get the problem switched.
    • -%} -% if ($results->{negative}{beginproblem}) { -
  • This problem contains the line TEXT(beginproblem()). This is no longer necessary and should be removed.
    • -%} -% if ($results->{negative}{context_texstrings}) { -
  • This problem contains the line Context()->texStrings;. This is no longer necessary and should be removed.
    • -%} -% if ($results->{negative}{oldtable}) { -
  • This problem contains the deprecated begintable command. This is not assessible and often cannot be - converted to hardcopy. This table should be written using nicetables or a PGML table.
    • -%} -% if ($results->{negative}{showPartialCorrect}) { -
  • This problem contains the line $showPartialCorrectAnswers = 1. This is enabled by default and needed only - if set to 0.
    • -% } -% if (!$pos->{solution}) { -
  • This problem does not have a solution. Consider adding one.
    • -% } -% if (!$pos->{randomness}) { -
  • This problem does not have randomness. Consider adding variables that take on random values.
    • -% } -% if ($results->{negative}{fun_cmp} || $results->{negative}{str_cmp} || $results->{negative}{num_cmp}) { -
  • This problem contains the functioins num_cmp, str_cmp or fun_cmp. - These are old ways of checking answers. These should be converted to MathObjects. -% } -% if ($results->{negative}{multiple_loadmacros}) { -
  • This problem contains two loadMacros function call. Combine the function - calls and make sure that all macros are needed for your problem.
    • -% } -% if ($results->{negative}{macros}{PGchoicemacros}) { -
  • This problem contains old versions of multiple choice. The sample problems - - Multiple Choice with Checkbox, - Multiple Choice with Popup and - Multiple Choice with Radio Buttons should be examined as well the macros: - parserPopUp.pl, - parserCheckboxList.pl and - parserRadioButtons.pl. - -
    • -% } -% if ($results->{negative}{macros}{PGgraphmacros}) { -
  • This problem uses PGgraphmacros a old plotting library. Consider using - Plots.pl - or PGtikz.pl
    • -%} -% if ($results->{negative}{lines_below_enddocument}) { -
  • There is content (code or other text), below the ENDDOCUMENT() line. Although this - is ignored, there shouldn't be content in this area.
    • -% } - -
- - +% Perl::Critic::Violation::set_format('%m at line %l, column %c.'); +% +
+

<%= maketext('PG Critic Results') %>

+ % my @goodResults = grep { ref($_->explanation) eq 'HASH' && ($_->explanation->{score} // 0) > 0 } @$results; + % if (@goodResults) { +

<%= maketext('The following are good aspects of this problem:') %>

+
    + % for (@goodResults) { +
  • +
    <%= $_->to_string %>
    +
    + <%= $_->explanation->{explanation} %> + See <%= link_to( + ($_->policy =~ s/^Perl::Critic::Policy:://r) + => pod_viewer => { filePath => 'lib/' . ($_->policy =~ s/::/\//gr) . '.pm' }, + target => '_blank' + ) %>. +
    +
    • + % } +
+ %} + % my @badResults = grep { ref($_->explanation) eq 'HASH' && ($_->explanation->{score} // 0) < 0 } @$results; + % if (@badResults) { +

<%= maketext('The following are things that can be improved:') %>

+
    + % for (@badResults) { +
  • +
    <%= $_->to_string %>
    +
    + <%= $_->explanation->{explanation} %> + <%== maketext( + 'See [_1].', + link_to( + ($_->policy =~ s/^Perl::Critic::Policy:://r) + => pod_viewer => { filePath => 'lib/' . ($_->policy =~ s/::/\//gr) . '.pm' }, + target => '_blank' + ) + ) =%> +
    + % if (ref($_->explanation->{sampleProblems}) eq 'ARRAY' && @{ $_->explanation->{sampleProblems} }) { +
    + <%== maketext('Related sample [plural,_1,problem]:', + scalar(@{ $_->explanation->{sampleProblems} })) %> + <%= c(map { + link_to( + $_->[0] => sample_problem_viewer => { filePath => $_->[1] }, + target => '_blank' + ) + } @{ $_->explanation->{sampleProblems} })->join(', ') =%> +
    + % } +
    • + % } +
+ %} + % my @badPerlResults = grep { !ref($_->explanation) } @$results; + % if (@badPerlResults) { +

<%= maketext('The following are general Perl issues that can be improved:') %>

+
    + % for (@badPerlResults) { +
  • +
    <%= $_->to_string %>
    +
    + See <%= link_to( + ($_->policy =~ s/^Perl::Critic::Policy:://r) => 'https://metacpan.org/pod/' . $_->policy, + target => '_blank' + ) %>. +
    +
    • + % } +
+ %}
diff --git a/templates/HelpFiles/InstructorPGProblemEditor.html.ep b/templates/HelpFiles/InstructorPGProblemEditor.html.ep index 8715387509..a0d85a64a6 100644 --- a/templates/HelpFiles/InstructorPGProblemEditor.html.ep +++ b/templates/HelpFiles/InstructorPGProblemEditor.html.ep @@ -156,19 +156,22 @@ . 'generating the PDF file using LaTeX.') =%>

- <%= maketext('You can also click "Edit Selected Theme" to edit a hardcopy theme. The new theme will be saved to ' - . 'the templates/hardcopyThemes folder.') =%> + <%= maketext('You can also click "Edit Selected Theme" to edit a hardcopy theme. The new theme will be saved ' + . 'to the templates/hardcopyThemes folder.') =%>

-
<%= maketext('Format Code') %>
+
<%= maketext('Code Maintenance') %>
- <%= maketext('Reformat the code using perltidy or a conversion to PGML. Using perltidy will change the code ' - . 'in the editor window, and save changes to the temporary file. In some cases (if the code contains ' - . 'backslashes or double tildes) this can result in odd spacing in the code. The convert to PGML ' - . 'feature changes the code in text blocks in the code to use PGML features. Generally the conversion of ' - . 'many of the formatting and LaTeX is performed correctly, however answer blanks need attention. In ' - . 'either case, make sure to inspect the formatted code, and edit further or revert if needed.') =%> + <%= maketext('Three code maintenance tools can be utilized. First, the code can be reformatted using perltidy. ' + . 'Using perltidy will change the code in the editor window, and save changes to the temporary file. In some ' + . 'cases (if the code contains backslashes or double tildes) this can result in odd spacing in the code. ' + . 'Second the code can be converted to PGML. This changes the code in text blocks to use PGML features. ' + . 'Generally the conversion of much of the formatting and LaTeX is performed correctly. However, answer ' + . 'blanks need attention. In any case, make sure to inspect the formatted code, and edit further or revert ' + . 'if needed. Third, the code can be analyzed by the "PG Critic Analyzer." This checks that the code uses ' + . 'modern features of PG, that it does not use old or deprecated features of PG, and offers advice on how ' + . 'to fix issues that are found.') =%>
<%= maketext('Save') %>
From bd647a1c6a478e3884c1abe9ea36253c2d4b9481 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 16 Jul 2025 06:31:00 -0500 Subject: [PATCH 3/3] Adjust for removal of "positive violations". --- lib/WebworkWebservice/ProblemActions.pm | 4 +- .../code_maintenance_form.html.ep | 8 ++-- .../PGProblemEditor/pg_critic.html.ep | 37 +++++-------------- .../InstructorPGProblemEditor.html.ep | 6 +-- 4 files changed, 18 insertions(+), 37 deletions(-) diff --git a/lib/WebworkWebservice/ProblemActions.pm b/lib/WebworkWebservice/ProblemActions.pm index f308e78beb..3305176361 100644 --- a/lib/WebworkWebservice/ProblemActions.pm +++ b/lib/WebworkWebservice/ProblemActions.pm @@ -172,8 +172,8 @@ sub runPGCritic { return { ra_out => { html => $self->c->render_to_string( - template => 'ContentGenerator/Instructor/PGProblemEditor/pg_critic', - results => [ critiquePGCode($params->{pgCode}) ] + template => 'ContentGenerator/Instructor/PGProblemEditor/pg_critic', + violations => [ critiquePGCode($params->{pgCode}) ] ) }, text => 'The script pg-critic has been run successfully.' diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/code_maintenance_form.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/code_maintenance_form.html.ep index a083a8edfe..f05df5d277 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor/code_maintenance_form.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/code_maintenance_form.html.ep @@ -34,11 +34,11 @@ <%= radio_button 'action.code_maintenance' => 'runPGCritic', id => 'action_code_maintenance_run_pgcritic', class => 'form-check-input'=%> <%= label_for 'action_code_maintenance_run_pgcritic', class => 'form-check-label', begin =%> - <%== maketext('Run the PG Critic Analyzer') =%> + <%== maketext('Analyze code with PG Critic') =%> <% end =%> - diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep index 3d6d1733de..7659b7453c 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep @@ -1,31 +1,12 @@ % Perl::Critic::Violation::set_format('%m at line %l, column %c.'); %
-

<%= maketext('PG Critic Results') %>

- % my @goodResults = grep { ref($_->explanation) eq 'HASH' && ($_->explanation->{score} // 0) > 0 } @$results; - % if (@goodResults) { -

<%= maketext('The following are good aspects of this problem:') %>

+

<%= maketext('PG Critic Violations') %>

+ % my @pgCriticViolations = grep { $_->policy =~ /^Perl::Critic::Policy::PG::/ } @$violations; + % if (@pgCriticViolations) { +

<%= maketext('The following PG issues should be fixed:') %>

    - % for (@goodResults) { -
  • -
    <%= $_->to_string %>
    -
    - <%= $_->explanation->{explanation} %> - See <%= link_to( - ($_->policy =~ s/^Perl::Critic::Policy:://r) - => pod_viewer => { filePath => 'lib/' . ($_->policy =~ s/::/\//gr) . '.pm' }, - target => '_blank' - ) %>. -
    -
    • - % } -
- %} - % my @badResults = grep { ref($_->explanation) eq 'HASH' && ($_->explanation->{score} // 0) < 0 } @$results; - % if (@badResults) { -

<%= maketext('The following are things that can be improved:') %>

-
    - % for (@badResults) { + % for (@pgCriticViolations) {
  • <%= $_->to_string %>
    @@ -55,11 +36,11 @@ % }
%} - % my @badPerlResults = grep { !ref($_->explanation) } @$results; - % if (@badPerlResults) { -

<%= maketext('The following are general Perl issues that can be improved:') %>

+ % my @perlCriticViolations = grep { $_->policy !~ /^Perl::Critic::Policy::PG::/ } @$violations; + % if (@perlCriticViolations) { +

<%= maketext('The following general Perl issues should be fixed:') %>

    - % for (@badPerlResults) { + % for (@perlCriticViolations) {
  • <%= $_->to_string %>
    diff --git a/templates/HelpFiles/InstructorPGProblemEditor.html.ep b/templates/HelpFiles/InstructorPGProblemEditor.html.ep index a0d85a64a6..66a4c7aa0d 100644 --- a/templates/HelpFiles/InstructorPGProblemEditor.html.ep +++ b/templates/HelpFiles/InstructorPGProblemEditor.html.ep @@ -169,9 +169,9 @@ . 'Second the code can be converted to PGML. This changes the code in text blocks to use PGML features. ' . 'Generally the conversion of much of the formatting and LaTeX is performed correctly. However, answer ' . 'blanks need attention. In any case, make sure to inspect the formatted code, and edit further or revert ' - . 'if needed. Third, the code can be analyzed by the "PG Critic Analyzer." This checks that the code uses ' - . 'modern features of PG, that it does not use old or deprecated features of PG, and offers advice on how ' - . 'to fix issues that are found.') =%> + . 'if needed. Third, the code can be analyzed by the "PG Critic." This checks that the code does not use ' + . 'old or deprecated features of PG and conforms to current best-practices in problem authoring, and offers ' + . 'suggestions on how to fix issues that are found.') =%>
    <%= maketext('Save') %>