Skip to content

Commit 1f5664c

Browse files
pstaabpdrgrice1
authored andcommitted
Update the pgeditor to use the pgcritic to analyze a problem.
1 parent de0407a commit 1f5664c

File tree

5 files changed

+208
-2
lines changed

5 files changed

+208
-2
lines changed

htdocs/js/PGProblemEditor/pgproblemeditor.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,30 @@
235235
.catch((err) => showMessage(`Error: ${err?.message ?? err}`));
236236
};
237237

238+
// Send a request to the server to run the PG critic in the CodeMirror editor.
239+
const runPGCritic = () => {
240+
const request_object = { courseID: document.getElementsByName('courseID')[0]?.value };
241+
242+
const user = document.getElementsByName('user')[0];
243+
if (user) request_object.user = user.value;
244+
const sessionKey = document.getElementsByName('key')[0];
245+
if (sessionKey) request_object.key = sessionKey.value;
246+
247+
request_object.rpc_command = 'runPGCritic';
248+
request_object.pgCode =
249+
webworkConfig?.pgCodeMirror?.source ?? document.getElementById('problemContents')?.value ?? '';
250+
251+
fetch(webserviceURL, { method: 'post', mode: 'same-origin', body: new URLSearchParams(request_object) })
252+
.then((response) => response.json())
253+
.then((data) => {
254+
renderArea.innerHTML = data.result_data.html;
255+
})
256+
.catch((err) => {
257+
console.log(err);
258+
showMessage(`Error: ${err?.message ?? err}`);
259+
});
260+
};
261+
238262
document.getElementById('take_action')?.addEventListener('click', async (e) => {
239263
if (document.getElementById('current_action')?.value === 'format_code') {
240264
e.preventDefault();
@@ -244,6 +268,8 @@
244268
document.querySelector('input[name="action.format_code"]:checked').value == 'convertCodeToPGML'
245269
) {
246270
convertCodeToPGML();
271+
} else if (document.querySelector('input[name="action.format_code"]:checked').value == 'runPGCritic') {
272+
runPGCritic();
247273
}
248274
return;
249275
}

lib/WebworkWebservice.pm

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ sub command_permission {
255255
putPastAnswer => 'problem_grader',
256256
tidyPGCode => 'access_instructor_tools',
257257
convertCodeToPGML => 'access_instructor_tools',
258+
runPGCritic => 'access_instructor_tools',
258259

259260
# WebworkWebservice::RenderProblem
260261
renderProblem => 'proctor_quiz_login',

lib/WebworkWebservice/ProblemActions.pm

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ use warnings;
66

77
use Data::Structure::Util qw(unbless);
88

9-
use WeBWorK::PG::Tidy qw(pgtidy);
10-
use WeBWorK::PG::ConvertToPGML qw(convertToPGML);
9+
use WeBWorK::PG::Tidy qw(pgtidy);
10+
use WeBWorK::PG::ConvertToPGML qw(convertToPGML);
11+
use WeBWorK::PG::PGProblemCritic qw(analyzePGcode);
1112

1213
sub getUserProblem {
1314
my ($invocant, $self, $params) = @_;
@@ -165,4 +166,21 @@ sub convertCodeToPGML {
165166

166167
}
167168

169+
sub runPGCritic {
170+
my ($invocant, $self, $params) = @_;
171+
my $pg_critic_results = analyzePGcode($params->{pgCode});
172+
173+
my $html_output = $self->c->render_to_string(
174+
template => 'ContentGenerator/Instructor/PGProblemEditor/pg_critic',
175+
results => $pg_critic_results
176+
);
177+
178+
return {
179+
ra_out => {
180+
html => $html_output
181+
},
182+
text => 'The script pg-critic has been run successfully.'
183+
};
184+
}
185+
168186
1;

templates/ContentGenerator/Instructor/PGProblemEditor/format_code_form.html.ep

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,18 @@
3030
<span class="visually-hidden"><%= maketext('PGML Conversion Help') %></span>
3131
</a>
3232
</div>
33+
<div class="form-check">
34+
<%= radio_button 'action.format_code' => 'runPGCritic',
35+
id => 'action_format_code_run_pgcritic', class => 'form-check-input'=%>
36+
<%= label_for 'action_format_code_run_pgcritic', class => 'form-check-label', begin =%>
37+
<%== maketext('Run the PG Critic Analyzer') =%>
38+
<% end =%>
39+
<a class="help-popup" data-bs-content="<%== maketext('This option runs the PG Critic '
40+
. 'code analyzer, which gives suggestions on using more modern PG constructs and '
41+
. 'ensure that you include important features. ') =%>"
42+
data-bs-placement="top" data-bs-toggle="popover" role="button">
43+
<i aria-hidden="true" class="fas fa-question-circle"></i>
44+
<span class="visually-hidden"><%= maketext('PG Critic Help') %></span>
45+
</a>
46+
</div>
3347
</div>
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
<div class="m-3" style="overflow: scroll">
2+
<h2>PG Critic Results</h2>
3+
4+
<h3>Metadata</h3>
5+
6+
<p>The following lists required metadata. If any is missing, the given tag must be filled in.
7+
However, make sure that the categories are correct, especially if the problem has been
8+
copied.</p>
9+
10+
% sub showIcon { my $show = shift;
11+
% return $show ? q!<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-check-square text-success" viewBox="0 0 16 16">
12+
% <path d="M14 1a1 1 0 0 1 1 1v12a1 1 0 0 1-1 1H2a1 1 0 0 1-1-1V2a1 1 0 0 1 1-1zM2 0a2 2 0 0 0-2 2v12a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V2a2 2 0 0 0-2-2z"/>
13+
% <path d="M10.97 4.97a.75.75 0 0 1 1.071 1.05l-3.992 4.99a.75.75 0 0 1-1.08.02L4.324 8.384a.75.75 0 1 1 1.06-1.06l2.094 2.093 3.473-4.425z"/>
14+
% </svg>! : q!<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-x-square text-danger" viewBox="0 0 16 16">
15+
% <path d="M14 1a1 1 0 0 1 1 1v12a1 1 0 0 1-1 1H2a1 1 0 0 1-1-1V2a1 1 0 0 1 1-1zM2 0a2 2 0 0 0-2 2v12a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V2a2 2 0 0 0-2-2z"/>
16+
% <path d="M4.646 4.646a.5.5 0 0 1 .708 0L8 7.293l2.646-2.647a.5.5 0 0 1 .708.708L8.707 8l2.647 2.646a.5.5 0 0 1-.708.708L8 8.707l-2.646 2.647a.5.5 0 0 1-.708-.708L7.293 8 4.646 5.354a.5.5 0 0 1 0-.708"/>
17+
% </svg>!;
18+
%}
19+
20+
<table class="table table-bordered">
21+
<tbody>
22+
<tr><th>DBsubject</th><td> <%== showIcon($results->{metadata}{DBsection}) %> </td></tr>
23+
<tr><th>DBchapter</th><td> <%== showIcon($results->{metadata}{DBchapter}) %> </td></tr>
24+
<tr><th>DBsection</th><td> <%== showIcon($results->{metadata}{DBsection}) %> </td></tr>
25+
<tr><th>Keywords</th><td> <%== showIcon($results->{metadata}{KEYWORDS}) %> </td></tr>
26+
</table>
27+
28+
% my $pos = $results->{positive};
29+
% if ($pos->{PGML} || $pos->{solution} || $pos->{hint}) {
30+
<h3>Good aspects of this problems are the following</h3>
31+
% }
32+
<table class="table table-bordered">
33+
<tbody>
34+
% if ($pos->{PGML}) {
35+
<tr><th>PGML</th><td>This problem uses PGML, the current preferred way to write problem (text), solution and hint
36+
blocks.</td></tr>
37+
% }
38+
% if ($pos->{solution}) {
39+
<tr><th>Solutions</th><td>This problem has a solution block. Every problem should have solutions that the
40+
student can view after the answer data. </td></tr>
41+
% }
42+
% if ($pos->{hint}) {
43+
<tr><th>Hints</th><td>This problem has a hint. This can be helpful for students after attempting the problem
44+
a few times (this can be set by the instructor).
45+
% }
46+
% if ($pos->{randomness}) {
47+
<tr><th>Randomness</th><td>This problem uses randomness. This is desired to give to a class of students, each
48+
of whom may have a different problem. </td>
49+
% }
50+
% # list of the positive contexts:
51+
% my @good_contexts = grep { $pos->{contexts}{$_} } keys %{$pos->{parsers}};
52+
% if (@good_contexts) {
53+
<tr><th>Modern Contexts</th><td>This problem uses the following modern contexts:
54+
<%= join(', ', @good_contexts) %> </td>
55+
% }
56+
% my @good_parsers = grep { $pos->{parsers}->{$_} } keys %{$pos->{parsers}};
57+
% if (@good_parsers) {
58+
<tr><th>Modern Parsers</th><td>This problem uses features of the following modern parsers:
59+
<%= join(', ', @good_parsers) %> </td>
60+
% }
61+
% my @good_macros = grep { $pos->{macros}->{$_} } keys %{$pos->{macros}};
62+
% if (@good_macros) {
63+
<tr><th>Modern Macros</th><td>This problem uses functionality from the following modern macros:
64+
<%= join(', ', @good_macros) %> </td>
65+
% }
66+
</tbody>
67+
</table>
68+
69+
70+
% if( scalar(@{$results->{deprecated_macros}}) > 0) {
71+
<h3>Deprecated Macros</h3>
72+
<p>This problem has the following deprecated macros: <%= join(', ',@{$results->{deprecated_macros}} ) %> </p>
73+
74+
<p>These should be removed from the problem in that these macros will be deleted from PG in a future
75+
version. The functions from these macros may be listed below to help aid in transitioning away from
76+
these macros. </p>
77+
% }
78+
79+
% my $has_bad_features = 0;
80+
% $has_bad_features += $results->{negative}{$_} for (keys %{$results->{negative}});
81+
82+
% if ($has_bad_features || !$pos->{solution}) {
83+
<h3>You can improve on the following:</h3>
84+
<p> There are features in this problem that contain old or deprecated features. The following
85+
list gives feedback of how the problem can be improved. </p>
86+
%}
87+
88+
<ul>
89+
% if ($results->{negative}{BEGIN_TEXT}) {
90+
<li>This problem contains older formatting blocks like BEGIN_TEXT. Consider use PGML.
91+
In the <em>Format Code</em> section of the PG Editor, the "Convert to PGML" should be used
92+
as a start to get the problem switched.</li>
93+
%}
94+
% if ($results->{negative}{beginproblem}) {
95+
<li>This problem contains the line <tt>TEXT(beginproblem())</tt>. This is no longer necessary and should be removed. </li>
96+
%}
97+
% if ($results->{negative}{context_texstrings}) {
98+
<li>This problem contains the line <tt>Context()->texStrings;</tt>. This is no longer necessary and should be removed. </li>
99+
%}
100+
% if ($results->{negative}{oldtable}) {
101+
<li>This problem contains the deprecated <tt>begintable</tt> command. This is not assessible and often cannot be
102+
converted to hardcopy. This table should be written using <tt>nicetables</tt> or a PGML table. </li>
103+
%}
104+
% if ($results->{negative}{showPartialCorrect}) {
105+
<li>This problem contains the line <tt>$showPartialCorrectAnswers = 1</tt>. This is enabled by default and needed only
106+
if set to 0.</li>
107+
% }
108+
% if (!$pos->{solution}) {
109+
<li>This problem does not have a solution. Consider adding one.</li>
110+
% }
111+
% if (!$pos->{randomness}) {
112+
<li>This problem does not have randomness. Consider adding variables that take on random values.</li>
113+
% }
114+
% if ($results->{negative}{fun_cmp} || $results->{negative}{str_cmp} || $results->{negative}{num_cmp}) {
115+
<li>This problem contains the functioins <tt>num_cmp</tt>, <tt>str_cmp</tt> or <tt>fun_cmp</tt>.
116+
These are old ways of checking answers. These should be converted to MathObjects.
117+
% }
118+
% if ($results->{negative}{multiple_loadmacros}) {
119+
<li>This problem contains two <tt>loadMacros</tt> function call. Combine the function
120+
calls and make sure that all macros are needed for your problem. </li>
121+
% }
122+
% if ($results->{negative}{macros}{PGchoicemacros}) {
123+
<li>This problem contains old versions of multiple choice. The sample problems
124+
<a target="_blank" href="https://openwebwork.github.io/pg-docs/sample-problems/Misc/MultipleChoiceCheckbox.html">
125+
Multiple Choice with Checkbox</a>, <a target="_blank" href="https://openwebwork.github.io/pg-docs/sample-problems/Misc/MultipleChoicePopup.html">
126+
Multiple Choice with Popup</a> and <a target="_blank" href="https://openwebwork.github.io/pg-docs/sample-problems/Misc/MultipleChoiceRadio.html">
127+
Multiple Choice with Radio Buttons</a> should be examined as well the macros:
128+
<a target="_blank" href="https://openwebwork.github.io/pg-docs/pod/pg/macros/parsers/parserPopUp.html">parserPopUp.pl</a>,
129+
<a target="_blank" href="https://openwebwork.github.io/pg-docs/pod/pg/macros/parsers/parserCheckboxList.html">parserCheckboxList.pl</a> and
130+
<a target="_blank" href="https://openwebwork.github.io/pg-docs/pod/pg/macros/parsers/parserRadioButtons.html">parserRadioButtons.pl</a>.
131+
132+
</li>
133+
% }
134+
% if ($results->{negative}{macros}{PGgraphmacros}) {
135+
<li>This problem uses <tt>PGgraphmacros</tt> a old plotting library. Consider using
136+
<a target="_blank" href="https://openwebwork.github.io/pg-docs/pod/pg/macros/graph/plots.html"><tt>Plots.pl</tt></a>
137+
or <a target="_blank" href="https://openwebwork.github.io/pg-docs/pod/pg/macros/graph/PGtikz.html"><tt>PGtikz.pl</tt></a></li>
138+
%}
139+
% if ($results->{negative}{lines_below_enddocument}) {
140+
<li>There is content (code or other text), below the <tt>ENDDOCUMENT()</tt> line. Although this
141+
is ignored, there shouldn't be content in this area.</li>
142+
% }
143+
144+
</ul>
145+
146+
147+
</div>

0 commit comments

Comments
 (0)