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: hint that code-blocks continue on next page (closes #3764) #3792

Merged
merged 2 commits into from
Jun 17, 2017

Conversation

jfbu
Copy link
Contributor

@jfbu jfbu commented May 23, 2017

This adds new key for 'sphinxsetup'. Provisory name is verbatimhintscontinuation.

The default is true and the effect is to print continuation hints when a code-block gets split in PDF output on multiple pages.

  • Feature

However, if default value of verbatimhintscontinuation was false there would be backward compatibility, so it could be merged in stable.

Relates

Currently the strings added to latex.tex_t have no translation. They are apart from an uppercase letter same as those from longtable.tex_t. See #3791 in this context.

@jfbu jfbu added type:enhancement enhance or introduce a new feature builder:latex labels May 23, 2017
@jfbu jfbu added this to the 1.7 milestone May 23, 2017
doc/latex.rst Outdated
@@ -218,6 +218,16 @@ Here are the currently available options together with their default values.
before or after, but this is accessible currently only by re-defining some
macros with complicated LaTeX syntax from :file:`sphinx.sty`.

``verbatimhintscontinuation``
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the name is not too good

doc/latex.rst Outdated
@@ -218,6 +218,16 @@ Here are the currently available options together with their default values.
before or after, but this is accessible currently only by re-defining some
macros with complicated LaTeX syntax from :file:`sphinx.sty`.

``verbatimhintscontinuation``
default ``true``. Boolean to specify if code blocks which happen to be split
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with default to false we could possibly merge in stable, as it is then only refactoring of non api code, with same output

@@ -30,6 +30,8 @@
<%= contentsname %>
<%= numfig_format %>
<%= pageautorefname %>
\newcommand{\sphinxVerbatimContinuedText}{\footnotesize(<%= _('continued from previous page') %>)}
\newcommand{\sphinxVerbatimContinuesText}{\footnotesize(<%= _('continues on next page') %>)}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

as indicated in #3791 it seems gettext did not pick strings from longtable templates so I guess one will have to be careful about latex template as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should I add string option to 'sphinxsetup' such as verbatimcontinued, verbatimcontinues ? with current code, user can use 'preamble' key and \renewcommand. With options, I need to then delay the definitions corresponding to the user options to at begin document. The latex file is then less understandable. Or, we move the lines above to before loading sphinx.sty

Copy link
Member

Choose a reason for hiding this comment

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

I think .sty file is usually used for modify styles and macros in LaTeX world.
So I feel preamble key is better to do that.

Note: it might be better to allow user's .sty file instead of preamble key.
Now the key became much important, but it is boring to maintain it in conf.py.

@@ -256,6 +256,7 @@
% verbatim
\DeclareBoolOption[true]{verbatimwithframe}
\DeclareBoolOption[true]{verbatimwrapslines}
\DeclareBoolOption[true]{verbatimhintscontinuation}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

see comment above about using rather false here

% let the framing obey the current indentation (adapted from framed.sty's code).
\hskip\@totalleftmargin
\hskip-\fboxsep\hskip-\fboxrule
#1{VerbatimBorderColor}{VerbatimColor}{#2}%
\spx@fcolorbox{VerbatimBorderColor}{VerbatimColor}{#1}{#2}{#3}%
Copy link
Contributor Author

Choose a reason for hiding this comment

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

formerly, special framing was used only to "glue" caption text to frame. Now we use it for continuation strings, hence always here \spx@fcolorbox. The extra parameters are the above and below text.

\def\spx@fcolorbox #1#2%
{\color@b@x {\fboxsep\z@\color{#1}\spx@VerbatimFBox}{\color{#2}}}%
\long\def\spx@fcolorbox #1#2#3#4%
{\color@b@x {\color{#1}\spx@VerbatimFBox{#3}{#4}}{\color{#2}}}%
Copy link
Contributor Author

Choose a reason for hiding this comment

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

formerly \fboxsep was set to zero, for the frame to touch the colored area

now, \spx@VerbatimFBox does not use \fboxsep so it is not needed to set it to zero

\def\sphinxVerbatimLastFrameCommand
{\spx@colorbox\sphinxVerbatimContinued{}}

\newcommand\sphinxVerbatimContinued{%
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this macro maybe customized by knowledgeable LaTeX user

@tk0miya
Copy link
Member

tk0miya commented May 25, 2017

I can't read the LaTeX macros well, so I will try to build PDF on my local later.
I will do it after 1.6.2 release. Please wait a moment :-)

@jfbu
Copy link
Contributor Author

jfbu commented May 25, 2017

sure I can wait ;-) a priori this is new feature hence for 1.7, although it could be merged into 1.6.x series if we set the default value of verbatimhintscontinuation to false (as a key for 'sphinxsetup') so that it remains transparent and introduces no change by default.

Even if we introduce at 1.7 possibility to use tcolorbox as discussed at #3790, I anticipate that it should be done as an option, keeping for a while the framed.sty use per default. Then this PR is not superfluous even if tcolorbox is usable at 1.7.

@jfbu
Copy link
Contributor Author

jfbu commented May 25, 2017

by the way, I forgot to mention the refactoring of LaTeX literal blocks in this PR makes possible now to get the code-block captions below rather than, as currently, above the frame.

@tk0miya
Copy link
Member

tk0miya commented Jun 4, 2017

Did you remove borders of code-block? Personally I loved it.

@jfbu
Copy link
Contributor Author

jfbu commented Jun 4, 2017

To remove borders of code-block, you need

    'sphinxsetup': 'verbatimwithframe=false',

for details http://www.sphinx-doc.org/en/stable/latex.html#the-sphinx-latex-style-package-options

@tk0miya
Copy link
Member

tk0miya commented Jun 4, 2017

In my local, the border is disabled by default. Even if I set verbatimwithframe=true.
Is this intended?

@jfbu
Copy link
Contributor Author

jfbu commented Jun 4, 2017

thanks for attentive review @tk0miya! this is not intended, and I confirmed it at my locale. I will fix that later... at some point I started testing with only background color. It worked fine in some earlier version with border. I must have done some latex bug afterwards. Thanks!

@jfbu
Copy link
Contributor Author

jfbu commented Jun 4, 2017

There was a missing brace pair at e1d7e3f#diff-f6432e57ae706b42894b186ffe699da0R727. As a result some \color command from inside the box for the background color contaminated the border color. Thus the border was there but with color white :-(

% note that the caption brings \abovecaptionskip top vertical space
\moveright\dimexpr\fboxrule+.5\wd\@tempboxa
\hb@xt@\z@{\hss\unhcopy\spx@VerbatimTitleBox\hss}\fi
\setbox\@tempboxa\hbox{{#3}}% contents with background color
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The extra brace pair: \hbox{{#3}} is important because #3 contains some \color command for the background. Without the extra braces, this colour contaminates the border. As the background is usually white, this means the border disappears.

Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

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

Confirmed. LGTM!

@jfbu
Copy link
Contributor Author

jfbu commented Jun 4, 2017

Thanks for confirmation @tk0miya ;-).

  1. I am not enthusiastic about my choice verbatimhintscontinuation. Perhaps verbatimwithcontinuationhints ? or verbatimpleaseturnover (refs), or verbatimwithpto...

  2. If we set the default value of this latex boolean to false, then output is 100% identical as earlier. Exact definition of non public latex macros is modified, but I doubt Sphinx users had custom ones. And if they do, they know well TeX and can adapt... Thus perhaps then this could be merged in stable, (it does not change anything and makes customization possible) and our docs would document that default will change from false to true at 1.7 ? (or we leave it false always).

Do you have opinion on 1. and 2. ?

@tk0miya
Copy link
Member

tk0miya commented Jun 11, 2017

  1. Sorry, I don't have another idea
  2. I agree to merge this into stable branch if it improves maintainability. You know we add new features to master branch basically, but it's okay to merge into stable if it prevents out development like merging operation (of cource, it should be stable enough)

@jfbu jfbu force-pushed the 3764_hint_verbatimcontinues branch from 1c33784 to 5673e46 Compare June 13, 2017 21:55
@jfbu jfbu changed the base branch from master to stable June 13, 2017 21:56
@jfbu
Copy link
Contributor Author

jfbu commented Jun 13, 2017

Thanks for review @tk0miya I have rebased on stable

  1. I changed ...hintscontinuation into ..hintsturnover. The former continuation was already used in macro names dealing with breaking of long code lines, so it was confusing.

  2. I changed default of latex boolean to false, so as to be able to rebase on stable

  3. I improved location of internationalized strings in latex document

  4. inner refactoring to have maximal customizability.

In my testing with default false boolean, this does not change anything to documents; but it does modify non public macros in sphinx.sty. Also, I will soon not be available for latex maintenance until September. Hence, in case of any doubt, it might be better to merge it in master for 1.7. Advantage of merging in 1.6.3 is that it will already be indicated in docs that verbatimhintsturnover boolean exists for 'sphinxsetup'. At 1.7. it will be true by default, but now false. Disadvantage of merging in 1.6.3 is that some inner LaTeX macros have been changed not to fix bugs but to add potential feature.

@jfbu jfbu force-pushed the 3764_hint_verbatimcontinues branch from 5673e46 to c2277ae Compare June 14, 2017 07:49
@jfbu
Copy link
Contributor Author

jfbu commented Jun 14, 2017

For 1.7 I will make a PR which will allow to customize:

  • position of code-block caption: before or after, left, center, or right
  • position of continuation hints: left, center, or right.
    In preparation for this I have amended this PR to again give private names to some latex macros, because they will need to be changed at 1.7.

And I have added \sphinxstylecodecontinued, \sphinxstylecodecontinues to specify font or color of the continuation hints.

At 1.7, the horizontal positioning will be customized via a 'sphinxsetup' option. The styling macros will need no change, they only decide of font and color and whether to use parentheses, etc, ... but not the position respective to code-block.

If no objection I will merge this to stable soon.

@jfbu jfbu merged commit c7914b3 into sphinx-doc:stable Jun 17, 2017
@jfbu
Copy link
Contributor Author

jfbu commented Jun 17, 2017

Thanks for review @tk0miya I have now merged and will make a PR on master for caption position of literal blocks.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
builder:latex type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants