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.

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

Implement TikZ images for problems #1030

Merged
merged 2 commits into from
Dec 28, 2019
Merged

Implement TikZ images for problems #1030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66b0aae
Add the TikZImage package to the list of loaded pg modules and add the
drgrice1 Sep 11, 2019
e45183e
Fix a missed instance of the change from $externalPrograms{curlCommand}
drgrice1 Dec 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions conf/defaults.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1238,6 +1238,7 @@ ${pg}{modules} = [
[qw(GD)],
[qw(AlgParser AlgParserWithImplicitExpand Expr ExprWithImplicitExpand utf8)],
[qw(AnswerHash AnswerEvaluator)],
[qw(TikZImage)],
[qw(WWPlot)], # required by Circle (and others)
[qw(Circle)],
[qw(Class::Accessor)],
Expand Down
7 changes: 6 additions & 1 deletion conf/site.conf.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ $externalPrograms{pdflatex} ="/usr/bin/pdflatex --shell-escape";

$externalPrograms{dvipng} ="/usr/bin/dvipng";

# In order to use imagemagick convert you need to change the rights for PDF files from
# "none" to "read" in the policy file /etc/ImageMagick-6/policy.xml. This has possible
# security implications for the server.
$externalPrograms{convert} ="/usr/bin/convert";

####################################################
# NetPBM - basic image manipulation utilities
# Most sites only need to configure $netpbm_prefix.
Expand All @@ -128,7 +133,7 @@ $externalPrograms{pngtopnm} = "$netpbm_prefix/pngtopnm";
####################################################
# set timeout time (-t 40 sec) to be less than timeout for problem (usually 60 seconds)
$externalPrograms{checkurl} = "/usr/bin/lwp-request -d -t 40 -mHEAD "; # or "/usr/local/bin/w3c -head "
$externalPrograms{curlCommand} = "/usr/bin/curl";
Copy link
Member

@mgage mgage Dec 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

the curlCommand is used in a number of places. It would be better to leave it for now (in addition to the
definition for {curl}

Copy link
Member

@mgage mgage Dec 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

/Volumes/WW_test/opt/webwork/webwork2_2018/lib/WeBWorK/PG.pm:211: $envir{externalCurlCommand} = $ce->{externalPrograms}->{curlCommand};
and
/Volumes/WW_test/opt/webwork/pg_2018/lib/PGcore.pm:769: $options->{curlCommand} = WeBWorK::PG::IO::curlCommand(); #FIXME just changed from alternate source

Also curlCommand occurs in several places in PG/IO.pm where it looks like the code is in the midst of a change over from curlCommand to curl. Using curlCommand within IO.pm and askSage() might
be ok -- it makes some of the commands to askSage clearer. I could go either way.

#FIXME change {curlCommand} to {curl}, here, and site.conf 

sub WeBWorK::PG::IO::curlCommand {
	# $CE->{externalPrograms}->{curlCommand};
	$CE->{externalPrograms}->{curl};
}

The other places, outside of those subroutines it seems to me that using {curl} as you have done is clearer. I no longer remember my exact thinking when I was last working on this code. What do you think? should we change all occurrences of curlCommand to curl?

Copy link
Member Author

@drgrice1 drgrice1 Dec 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I could put it back if you think that would be safer. I was just trying to complete what was done in your tikz pull request. It does seem the naming was inconsistent relative to the other external program variables. I had thought I changed all instances of {curl command} to {curl}. Perhaps I missed some.

Copy link
Member

Choose a reason for hiding this comment

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

On closer inspection I think you only missed one. The line
211 in PG.pm . This defines a variable in the PG environment called
externalCurlCommand. I don't see where that is ever used.

I think I'm in favor of commenting out line 211 in PG.pm (see whether that ever causes problems).

I've decided I'm ok with continuing to use curlCommand inside askSage and IO.pm since I believe my idea there was that we might want to add additional switches to the bare curl command and therefore created a front end subroutine to make it easier to modify the bare curl command if we needed to. I'm willing to change my mind on that if you think that is over complicating things.

I was trying to make the assignments in site.conf more uniform -- and assigning {curlCommand} stuck out -- so you were following my initial intentions there.

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 see. I did miss line 211 of PG.pm. We can comment that out, or just adjust it to be {curl}, or put everything back the way it was. That isn't part of the tikz image work, and I don't want it to interfere with getting that in. Just let me know what you prefer.

$externalPrograms{curl} = "/usr/bin/curl";

####################################################
# image conversions utiltiies
Expand Down
2 changes: 1 addition & 1 deletion lib/WeBWorK/PG.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ sub defineProblemEnvir {
$envir{externalPng2EpsPath} = $ce->{externalPrograms}->{png2eps};
$envir{externalGif2PngPath} = $ce->{externalPrograms}->{gif2png};
$envir{externalCheckUrl} = $ce->{externalPrograms}->{checkurl};
$envir{externalCurlCommand} = $ce->{externalPrograms}->{curlCommand};
#$envir{externalCurlCommand} = $ce->{externalPrograms}->{curl};
# Directories and URLs
# REMOVED: courseName
# ADDED: dvipngTempDir
Expand Down