Opened 8 years ago

Closed 7 years ago

#11795 closed enhancement (fixed)

Easily customize different viewers for PNG, DVI, PDF

Reported by: leif Owned by: was
Priority: major Milestone: sage-5.6
Component: user interface Keywords: plot browser
Cc: kcrisman Merged in: sage-5.6.beta1
Authors: John Palmieri Reviewers: Nathann Cohen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by ncohen)

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 ncohen 7 years ago.
trac_11795.v2.patch (16.3 KB) - added by jhpalmieri 7 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 8 years ago by jhpalmieri

Oh, good, more environment variables. ;)

comment:2 Changed 8 years ago by jhpalmieri

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 8 years ago by jhpalmieri

  • Authors set to John Palmieri
  • Description modified (diff)
  • Status changed from new to needs_review
  • Summary changed from Support environment variables for different viewers (e.g. PNG, DVI, PDF) to Easily customize different viewers for PNG, DVI, PDF

Okay, this has doctests and is ready for review.

comment:4 Changed 8 years ago by kcrisman

  • Cc kcrisman added

comment:5 Changed 7 years ago by ncohen

Just rebased the patch on 5.5.rc0

Changed 7 years ago by ncohen

comment:6 Changed 7 years ago by ncohen

  • Status changed from needs_review to needs_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 7 years ago by ncohen

  • Status changed from needs_work to needs_info

Changed 7 years ago by jhpalmieri

comment:8 Changed 7 years ago by jhpalmieri

  • Description modified (diff)
  • Status changed from needs_info to needs_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 7 years ago by ncohen

  • Description modified (diff)
  • Status changed from needs_review to positive_review

Well, then...

Thank you again ! :-)

Nathann

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

comment:10 Changed 7 years ago by jdemeyer

  • Reviewers set to Nathann Cohen

comment:11 Changed 7 years ago by 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 7 years ago by ncohen

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 7 years ago by 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 7 years ago by ncohen

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 7 years ago by jdemeyer

  • Merged in set to sage-5.6.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.