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

Latex improvements #1024

Merged
merged 11 commits into from May 15, 2019
Merged

Latex improvements #1024

merged 11 commits into from May 15, 2019

Conversation

t-makaro
Copy link
Contributor

@t-makaro t-makaro commented May 13, 2019

The follow up PR to #992.

  • Slightly better title spacing. (The gap between title/author/date is smaller. Added usepackage{titling}))
  • Better spacing between cells. (Removed \tcbset{nobeforeafter})
  • Paragraphs are no longer auto-indented like in markdown. (Added \usepackage{parskip})
  • Better prompt font, and spacing (using tcolorbox's lengths)
  • Fixes Hidden \includegraphics[]{} latex call #340. I was unable to use the \setkeys option to fix this, since it had a side effect of shrinking adjustimage boxes. However, by adding the Export flag to the adjustbox package it exports a new version of includegraphics that still allows use of all the keys whilst still setting a maximum size for images.
  • Stop alt-text from being used as an image caption. Apparently, before, it was just removing the caption label but not the caption as the comments described. This PR now supersedes [WIP] Strict placement of images #990, and images in the pdf are now placed like in the notebook.

Before:
image

After:
image

@t-makaro t-makaro requested a review from MSeal May 13, 2019 23:43
@t-makaro
Copy link
Contributor Author

t-makaro commented May 13, 2019

Also: Spacing between cells:
Before:
image

After:
image

This applies to text before/after cells too.

CC @mgeier

@t-makaro t-makaro requested review from mpacer and removed request for mpacer May 14, 2019 00:15
Copy link
Contributor

@MSeal MSeal left a comment

Choose a reason for hiding this comment

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

It looks good to me, but I have a hard to evaluating for potential latex cornercases. What's the risk factor in your opinion? Should we get another more latex knowledgeable review or do you feel pretty confident this is the right approach? If it's the later I can merge this now.

@t-makaro
Copy link
Contributor Author

t-makaro commented May 15, 2019

I believe it is the right approach.

The adjustbox package has an ability to add features to the original includegraphics (with a lower case export instead of Export), but the documentation explicitly warns that not all features may work with that method. This should actually be more feature filled and customizable than the original includegraphics.

All of the options in the original includegraphics should still work fine. I tested setting the width key to larger than the 90% maximum that I've set, and it will ignore the maximum if asked too which is good for people using custom templates.

My bigger worry is if someone made a template that used \Oldincludegraphics. This would break their template. I could re-add the \Oldincludegraphics definition for compatibility.

@MSeal
Copy link
Contributor

MSeal commented May 15, 2019

Ahh the \Oldincludegraphics might be a valid concern. I've seen a lot of threads over the past two releases where people reference that in relation to other issue.

We could keep the backwards compatibility for the next patch release but note in the release notes that we'll remove that compatibility in 6.x?

@t-makaro
Copy link
Contributor Author

@MSeal I managed to squeeze in one more fix. It's good to merge once tests pass.

@t-makaro t-makaro requested a review from MSeal May 15, 2019 23:21
@MSeal MSeal merged commit 63d1667 into jupyter:master May 15, 2019
@t-makaro t-makaro deleted the latex_improvements branch May 15, 2019 23:33
@MSeal MSeal added this to the 5.6 milestone Jul 30, 2019
@meeseeksmachine
Copy link

This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/nbconvert-5-6-0-release/1867/1

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

Successfully merging this pull request may close these issues.

Hidden \includegraphics[]{} latex call
3 participants