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.

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

Make "tightpage=True" the default behavior for view #21929

Closed
mo271 opened this issue Nov 22, 2016 · 28 comments
Closed

Make "tightpage=True" the default behavior for view #21929

mo271 opened this issue Nov 22, 2016 · 28 comments

Comments

@mo271
Copy link
Contributor

mo271 commented Nov 22, 2016

The original implementation of view, produces a pdf in a4 format,
regardless of the typeset object. #6591 introduced the tightpage
option which produces a document whose size matches the typeset
object.

Since then, the tightpage=True option has grown popular; in fact
most of the time, this is the desirable choice. Therefore this ticket
makes tightpage=True the default.

Note: in some cases, the current implementation of tightpage crops the
border of the picture. This will be fixed in a separate ticket to make
tightpage even more appealing.

CC: @jplab @nthiery

Component: misc

Keywords: days79, latex

Author: Moritz Firsching

Branch/Commit: 1bf4697

Reviewer: Jean-Philippe Labbé

Issue created by migration from https://trac.sagemath.org/ticket/21929

@mo271 mo271 added this to the sage-7.5 milestone Nov 22, 2016
@mo271
Copy link
Contributor Author

mo271 commented Nov 22, 2016

Commit: 7f932ca

@mo271
Copy link
Contributor Author

mo271 commented Nov 22, 2016

New commits:

7f932caFixing ticket 21929

@mo271
Copy link
Contributor Author

mo271 commented Nov 22, 2016

Branch: u/moritz/ticket/21929

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 22, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

15d6d2fChanged documentation to indicate the new default behavior

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 22, 2016

Changed commit from 7f932ca to 15d6d2f

@mo271
Copy link
Contributor Author

mo271 commented Nov 22, 2016

comment:3

I would like to remove the many occurrences of tightpage=True in various files to reflect the new defaults.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 22, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

800b051removed occurances of 'tightpage=true'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 22, 2016

Changed commit from 15d6d2f to 800b051

@mo271
Copy link
Contributor Author

mo271 commented Nov 22, 2016

Author: moritz

@jplab
Copy link

jplab commented Nov 23, 2016

comment:6

Hi Moritz,

Did you make a circular search do look for all instances of "tightpage=(T)true"?

@jplab
Copy link

jplab commented Nov 23, 2016

Reviewer: Jean-Philippe Labbé

@jplab
Copy link

jplab commented Nov 23, 2016

Changed author from moritz to Moritz Firsching

@jplab
Copy link

jplab commented Nov 23, 2016

comment:8

Hi Moritz,

Could you also adapt the text in the documentation of view concerning the option "tightpage" so that it makes sense with the updated default value. For example, say what it does when it is set to "False".

@jplab
Copy link

jplab commented Nov 23, 2016

comment:9

All test passed on Sage7.5.b3.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 23, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

bf273famore improvements in the documentation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 23, 2016

Changed commit from 800b051 to bf273fa

@mo271
Copy link
Contributor Author

mo271 commented Nov 23, 2016

comment:11

I searched for all instances of tightpage=True and hopefully didn't miss any.

The last commit addresses the changes in the documentation you suggest.


New commits:

bf273famore improvements in the documentation

@jplab
Copy link

jplab commented Nov 23, 2016

comment:12

Great! All test passed.

I would like to have a second opinion. Nicolas, anything to say about the patch?

This looks good for a positive review on my part.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 23, 2016

Changed commit from bf273fa to 397cd78

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 23, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

397cd78fixed one more occurance

@jplab
Copy link

jplab commented Nov 23, 2016

comment:14

Dear Moritz,

I did a pep8 check on latex.py. Could you remove the "blanklines containing whitespaces" and "trailing whitespaces" in line 2101 and 2103, 2118 of latex.py.

JP

@nthiery

This comment has been minimized.

@nthiery nthiery changed the title make "tightpage" the default behavior for view Make "tightpage=True" the default behavior for view Nov 23, 2016
@nthiery

This comment has been minimized.

@nthiery
Copy link
Contributor

nthiery commented Nov 23, 2016

comment:17

For the record: we made a quick poll here at Sage Days 79, and everybody who cared voted to make tightpage=True the default.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 23, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

1bf4697removed some whitespace

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 23, 2016

Changed commit from 397cd78 to 1bf4697

@jplab
Copy link

jplab commented Nov 23, 2016

comment:19

Looks ok to me now. I'm setting it to positive review.

@vbraun
Copy link
Member

vbraun commented Nov 27, 2016

Changed branch from u/moritz/ticket/21929 to 1bf4697

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants