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

varwidth first aid of June 8 2021 perhaps not enough #586

Closed
jfbu opened this issue Jun 16, 2021 · 12 comments
Closed

varwidth first aid of June 8 2021 perhaps not enough #586

jfbu opened this issue Jun 16, 2021 · 12 comments

Comments

@jfbu
Copy link

jfbu commented Jun 16, 2021

Brief outline of the bug

At https://github.com/sphinx-doc/sphinx we support producing tables with complex merged cells combining multiple columns and multiple rows in a way going beyond what is available via available packages (contents possibly using verbatim like environment, multiple paragraphs, lists). One part of this is relying on usage of varwidth. The #583 hit us but #584 fix does not solve the issue for us. sphinx-doc/sphinx#9315

The extra whitespace which appeared above now appears below. Below, I will include screenshot of a TL2019 build and of a TL2021 build including the #584 first-aid. Prior to the first-aid the extra whitespace appeared on top, now it is below, but I have no screenshot because I updated my installation and don't want to go through rebuilding format (not sure if rollback applies to first-aid, no time to check; side note: the first-aid mechanism seems to leave no trace in the log except if any first-aid update trigger a "PL" stepping? I am not sure if "PL1" is enough to tell that the first-aid dated 2021/06/08 was included or not).

I have reduced it to an almost LaTeX issue, "almost" because it is using \vbox primitive. For lack of time I have not at all looked at what is mechanism and I have not tried to find some pure LaTeX without explicit usage of \vbox. The rationale here of the code was to ensure some depth without having to worry about \parskip setting. (and the template can not simply inserts a \strut because material above may be vmode or end in a \par not easy to otherwise control).

Minimal example showing the bug

\RequirePackage{latexbug}
\documentclass{article}
\usepackage{varwidth}
\begin{document}

\fbox{\begin{varwidth}[t]{3cm}
some text
\par
\vskip-\baselineskip\vbox{\hbox{\strut}}%
\end{varwidth}}%

\end{document}

Log file (required) and possibly PDF file

TL2019:

Capture d’écran 2021-06-16 à 09 42 00

TL2021, first-aid 2021/06/08

Capture d’écran 2021-06-16 à 09 44 08

Prior to first-aid extra whitespace on top, not on bottom.

varwidthissue.log

@u-fischer
Copy link
Member

Well the problem is more the primitive \vskip. Your example works with \vspace{-\baselineskip}.

These alternatives compile for me with the expected result:

   \vspace{-\baselineskip}\strut

   \vskip -\baselineskip \vskip0pt \vbox{\hbox{\strut}\par}

Side remark: As described in ltpara-doc.pdf it is important to correctly end a \vbox with an explicit \par. It can put the paragraph hooks out-of-sync if this is missing and this can disturb the tagging of paragraphs.

@jfbu
Copy link
Author

jfbu commented Jun 16, 2021

Side remark: As described in ltpara-doc.pdf it is important to correctly end a \vbox with an explicit \par. It can put the paragraph hooks out-of-sync if this is missing and this can disturb the tagging of paragraphs.

You mean a \par inside the \vbox right? Because here the \strut starts a paragraph?

@jfbu
Copy link
Author

jfbu commented Jun 16, 2021

Side remark: As described in ltpara-doc.pdf it is important to correctly end a \vbox with an explicit \par. It can put the paragraph hooks out-of-sync if this is missing and this can disturb the tagging of paragraphs.

You mean a \par inside the \vbox right? Because here the \strut starts a paragraph?

Sorry got confused. I actually use \hbox{\strut} to not trigger paragraph builder.

@jfbu
Copy link
Author

jfbu commented Jun 16, 2021

Hence I don't understand your remark about \vbox versus \par.

@jfbu
Copy link
Author

jfbu commented Jun 16, 2021

@u-fischer Indeed \vskip-\baselineskip\vskip0pt\vbox{\hbox{\strut}}% works for me also in my original context of the more complex examples at sphinx-doc/sphinx#9315. Thanks for the workaround.

@u-fischer
Copy link
Member

Hence I don't understand your remark about \vbox versus \par.

I'm not sure if it matters here, it is only something one has to be careful with. varwidth seems to mess up the paragraph numbers anyway :-(

@FrankMittelbach
Copy link
Member

In a nutshell, varwidth makes some assumptions of how vertical spaces show up in a vertical list (especially after paragraphs) and with the 2021 release that assumption is no longer true, hence the first aid. However, the way that firstaid was written and the way varwidth unscrambles the vertical list this now means that stray vskips get lost (while LaTeX ones work because they are protected from immediate removal).

You could try this, I think it will bring back the old behavior faithfully:

\def\@vwid@sift{%
  \skip@\lastskip\unskip
  \ifdim\lastskip=\z@\unskip\fi         % <---- the first aid here (not just unskip)
  \dimen@\lastkern\unkern
  \count@\lastpenalty\unpenalty
  \setbox\z@\lastbox
  \ifvoid\z@ \advance\sift@deathcycles\@ne \else \sift@deathcycles\z@ \fi
  \ifnum\sift@deathcycles>33 
    \let\@vwid@sift\relax
    \PackageWarning{varwidth}{Failed to reprocess entire contents}%
  \fi
  \ifnum\count@=\@vwid@preeqp \@vwid@eqmodefalse\fi
  \ifnum\count@=\@vwid@posteqp \@vwid@eqmodetrue\fi
  \ifnum\count@=\@vwid@toppen % finished
    \let\@vwid@sift\relax
  \else\ifnum\count@=\@vwid@offsets
    \@vwid@setoffsets
  \else
    \ifnum\count@=\@vwid@postw
    \else
      \@vwid@resetb % reset box \z@ or measure it
    \fi
    \@vwid@append
  \fi\fi
  \@vwid@sift}%

What also works is guarding the stray vskip by following it with a \vskip\z@ in your example (regardless of the above fix so for now this is the nest workaround).

I guess this means indeed improving the firstaid is in order (or even better that Donald fixes varwidth) but this has to waiting until after my vacation.

@FrankMittelbach FrankMittelbach self-assigned this Jun 16, 2021
@jfbu
Copy link
Author

jfbu commented Jun 16, 2021

You could try this, I think it will bring back the old behavior faithfully:

Yes, it does appear to achieve that indeed in my original use case. For time being I prepared to add \vskip0pt which is not burden to us, but would still leave me have to check through codebase whatever stray \vskip there can be. So ultimately the second version of first-aid is indeed useful to us in original context if we do not have to change our mark-up.

@FrankMittelbach FrankMittelbach added this to Pool (unscheduled issues) in upcoming LaTeX2e releases via automation Jun 22, 2021
@FrankMittelbach FrankMittelbach added this to the hotfix milestone Jun 22, 2021
@FrankMittelbach
Copy link
Member

goes into next update of first-aid

@FrankMittelbach FrankMittelbach changed the title varwidth first aid of June 8 2021 perhaps not enoughh varwidth first aid of June 8 2021 perhaps not enough Jun 22, 2021
FrankMittelbach added a commit that referenced this issue Jun 22, 2021
expecting #591 to change the version number
@FrankMittelbach FrankMittelbach moved this from Pool (unscheduled issues) to Done in upcoming LaTeX2e releases Jun 24, 2021
@FrankMittelbach
Copy link
Member

hotfix on its way to CTAN (more or less)

@jfbu
Copy link
Author

jfbu commented Jun 26, 2021

Thanks, this fixes sphinx-doc/sphinx#9315 upstream issue at least for the case reported in that ticket.

@FrankMittelbach
Copy link
Member

welcome. Donald is informed and I would expect to see an update to varwidth eventually showing up so that this "first-aid" could then be dropped again.

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

No branches or pull requests

3 participants