Opened 11 years ago

Closed 10 years ago

#11795 closed enhancement (fixed)

Easily customize different viewers for PNG, DVI, PDF

Reported by: Leif Leonhardy Owned by: William Stein
Priority: major Milestone: sage-5.6
Component: user interface Keywords: plot browser
Cc: Karl-Dieter Crisman Merged in: sage-5.6.beta1
Authors: John Palmieri Reviewers: Nathann Cohen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Nathann Cohen)

Currently, all viewers default to $SAGE_BROWSER if it is set, $BROWSER or some other system-specific defaults (like xdg-open, if present) otherwise.

The original proposal on this ticket was to support environment variables for what is currently

  • sage.misc.viewer.DVI_VIEWER,
  • sage.misc.viewer.PDF_VIEWER, and
  • sage.misc.viewer.PNG_VIEWER.

I think instead that if the user wants to change the defaults, then that should happen in their init.sage file. The current patch allows them to add lines like these:

from sage.misc.viewer import viewer
viewer.browser('open -a /Applications/Chrome.app')
viewer.png_viewer('display')
viewer.pdf_viewer('acroread')
viewer.dvi_viewer('/usr/bin/xdvi')

(or of course to use these interactively from the command line) and then the appropriate program will be used.


Apply :

Attachments (2)

trac_11795-rev.patch (1.7 KB) - added by Nathann Cohen 10 years ago.
trac_11795.v2.patch (16.3 KB) - added by John Palmieri 10 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 11 years ago by John Palmieri

Oh, good, more environment variables. ;)

comment:2 Changed 11 years ago by John Palmieri

On further thought, I think this is a bad idea. These variables are only used within Sage, so rather than environment variables, they should be set in .sage/init.sage. I'm attaching a patch (which needs work: no doctests and not really tested at all), which allows for this sort of behavior:

# init.sage file
from sage.misc.viewer import viewer
viewer.pdf_viewer('open -a /Applications/Adobe\ Reader.app')

Then if a is some Sage object, this opens up Adobe Reader on my Mac:

view(a, pdflatex=True)

(The attached patch also fixes a few bugs in latex.py so that this actually works.)

Opinions about using init.sage instead of environment variables?

comment:3 Changed 11 years ago by John Palmieri

Authors: John Palmieri
Description: modified (diff)
Status: newneeds_review
Summary: Support environment variables for different viewers (e.g. PNG, DVI, PDF)Easily customize different viewers for PNG, DVI, PDF

Okay, this has doctests and is ready for review.

comment:4 Changed 11 years ago by Karl-Dieter Crisman

Cc: Karl-Dieter Crisman added

comment:5 Changed 10 years ago by Nathann Cohen

Just rebased the patch on 5.5.rc0

Changed 10 years ago by Nathann Cohen

Attachment: trac_11795-rev.patch added

comment:6 Changed 10 years ago by Nathann Cohen

Status: needs_reviewneeds_work

ellooooooooooooo John !!

Thank you very much for this patch ! I had actually begun to write a (much worse) version of it, then noticed you had already done the job ! I joined a patch adding unimportant details to yours, and there's one question keeping me from giving a positive review to your patch immediately : why isn't _viewer_prefs just a dictionary ?

This should get in quickly :-)

Nathann

comment:7 Changed 10 years ago by Nathann Cohen

Status: needs_workneeds_info

Changed 10 years ago by John Palmieri

Attachment: trac_11795.v2.patch added

comment:8 Changed 10 years ago by John Palmieri

Description: modified (diff)
Status: needs_infoneeds_review

I don't know why _viewer_prefs is not just a dictionary. Maybe that class used to do more things in some prehistoric version of the patch? Anyway, I changed it so it's just a dictionary now.

comment:9 Changed 10 years ago by Nathann Cohen

Description: modified (diff)
Status: needs_reviewpositive_review

Well, then...

Thank you again ! :-)

Nathann

Apply trac_11795.v2.patch, trac_11795-rev.patch

comment:10 Changed 10 years ago by Jeroen Demeyer

Reviewers: Nathann Cohen

comment:11 Changed 10 years ago by Greg Minshall

hi. fwiw, i downloaded an applied these two patches on my macosx 10.8 system (using fink). i added the following to my init.sage:

from sage.misc.viewer import viewer
viewer.browser('/sw/bin/w3m')
viewer.dvi_viewer('/sw/bin/xdvi')
viewer.pdf_viewer('/sw/bin/xpdf')
viewer.png_viewer('/sw/bin/display')

and ran simple tests. everything worked just fine. (i had a separate issue with error reporting for non-existent viewers, but i don't think that's in the scope of this patch.)

*i* would also like the ability to get plot3d to use other than jmol. i suspect that's also out of scope for this patch. so, i'm basically happy. cheers.

comment:12 Changed 10 years ago by Nathann Cohen

Helloooooooooooo !!

Well, as this one has been reviewed and fixes something already let's not disturb it. This being said, if you create a ticket to fix your problem do add me as a Cc, if I understand it the review will be fast :-)

Nathann

comment:13 Changed 10 years ago by Greg Minshall

hello, back.

okay. i've added #13842 (give an error or warning if user specified program does not exist) and #13843 (allow configuration of 3D plot program).

i wasn't sure of who to add as CCs, etc. i'm happy to be educated in the way of the sage...

cheers.

comment:14 Changed 10 years ago by Nathann Cohen

Hellooooooooooo !!!

Well, you can add as a CC whoever may feel involved by the patch. Like me, as I reviewed this, or John (who did all the smart work). This being said, it is very likely that these tickets will remain open for a while unless you write a patch yourself and want it to be reviewed :-)

Nathann

comment:15 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.6.beta1
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.