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

changing font series defaults #306

Closed
schlcht opened this issue Mar 9, 2020 · 10 comments
Closed

changing font series defaults #306

schlcht opened this issue Mar 9, 2020 · 10 comments

Comments

@schlcht
Copy link

schlcht commented Mar 9, 2020

Brief outline of the bug

The new font series management seems to make it impossible to change the default series other than, well, through the new font series management. Modifying \bfdefault or \mddefault won't have any effect anymore because the values in \bfseries@rm etc. will always take precedence. Is this really the intended behaviour? I sense some major compatibility issues here.

Secondly, specifying \DeclareFontSeriesDefault without the optional argument won't be effective, either (for the same reason), which actually makes the optional argument mandatory.

Or am I missing something completely?

Minimal example showing the bug

\RequirePackage{latexbug}
\documentclass{article}
\renewcommand\rmdefault{EBGaramond-LF}
\renewcommand\bfdefault{sb}
\DeclareFontSeriesDefault{bf}{sb}
%\DeclareFontSeriesDefault[rm]{bf}{sb} % only this works
\begin{document}
\textbf{\expandafter\meaning\the\font}
\end{document}

Log file (required) and possibly PDF file

test.log

@FrankMittelbach
Copy link
Member

Secondly, specifying \DeclareFontSeriesDefault without the optional argument won't be effective, either (for the same reason), which actually makes the optional argument mandatory.

Or am I missing something completely?

no you don't. That's a bug. The logic to work around the fact the CM/LM need bx while more or less any other font needs b is obviously not quite correct, i it doesn't take a change of \bfdefault into account. Technically using \renewcommand\bfdefault{sb} or \DeclareFontSeriesDefault{bf}{sb} (which is the same in other words) is probably most of the time not a good idea as it means that sf and tt also need to support that series, but it should of course work.

So here is a possible patch -- not necessarily the one that goes into the kernel in the end but give it a try:

\makeatletter
\def\init@series@setup{%
  \edef\reserved@a{\bfdefault}%
  \def\reservedb{b}%
 \ifx\reserved@a\reserved@b
  \ifx\bfseries@rm@kernel\bfseries@rm
    \expandafter\in@\expandafter{\rmdefault}{cmr,cmss,cmtt,lcmss,lcmtt,lmr,lmss,lmtt}%
    \ifin@ \else \def\bfseries@rm{b}\fi\fi
  \ifx\bfseries@sf@kernel\bfseries@sf
    \expandafter\in@\expandafter{\sfdefault}{cmr,cmss,cmtt,lcmss,lcmtt,lmr,lmss,lmtt}%
    \ifin@ \else \def\bfseries@sf{b}\fi\fi
  \ifx\bfseries@tt@kernel\bfseries@tt
    \expandafter\in@\expandafter{\ttdefault}{cmr,cmss,cmtt,lcmss,lcmtt,lmr,lmss,lmtt}%
    \ifin@ \else \def\bfseries@tt{b}\fi\fi
 \else
  \ifx\bfseries@rm@kernel\bfseries@rm
    \let\bfseries@rm\reserved@a
  \fi
  \ifx\bfseries@sf@kernel\bfseries@sf
    \let\bfseries@sf\reserved@a
  \fi
  \ifx\bfseries@tt@kernel\bfseries@tt
   \let\bfseries@tt\reserved@a
  \fi
 \fi
 %   
  \expand@font@defaults
  \ifx\famdef@ult\rmdef@ult      \rmfamily
  \else\ifx\famdef@ult\sfdef@ult \sffamily
  \else\ifx\famdef@ult\ttdef@ult \ttfamily
  \fi\fi\fi
}%
\makeatother

\documentclass{article}
\renewcommand\rmdefault{EBGaramond-LF}
\renewcommand\bfdefault{sb}
\DeclareFontSeriesDefault{bf}{sb}  % that is really the same as the line before

%\DeclareFontSeriesDefault[rm]{bf}{sb} % only this works
\begin{document}



\textbf{\expandafter\meaning\the\font}

\sffamily
\textbf{\expandafter\meaning\the\font}  % will fail if sf is CMSS but that's correct

\stop

@schlcht
Copy link
Author

schlcht commented Mar 9, 2020

Thanks. This would still fail, however, when the change happens inside the document. I'm aware that \DeclareFontSeriesDefault with its camel case looks like it should only be used in the preamble, but it's not actually \@onlypreamble'd. Also, my real-life case was a change of \bfdefault just for the TOC.

@FrankMittelbach
Copy link
Member

I guess there is no way to avoid getting some edge cases that are behaving differently when you extend functionality, but maybe the following logic will make old and new work together well enough.

  • going forward the setting of \bfdefault directly is deprecated (but remains possible for old documents)
  • the default for a meta family (rm/sf/tt) should be set using \DeclareFontSeriesDefault[<meta>]{bf}{...}
  • the default for any other font (not one of the meta families) should be set using \DeclareFontSeriesDefault{bf}{...}
  • same goes for \mddefault obviously

The processing logic is then:

  • when using \DeclareFontSeriesDefault only the part that is set up is being altered
  • when using \renewcommand\bfdefault{...} all existing meta family defaults are reset to \bfdefault as well (technically that is going to happen at the next invocation of \bfseries not at the time the \renewcommand is parsed)

Does that make sense or did I overlook a case?

@FrankMittelbach
Copy link
Member

I should add that I think it is next to impossible (with reasonable effort) to detect the case that \bfdefault was changed to itself so if that happen I guess no resetting of the meta family defaults will happen.

FrankMittelbach added a commit that referenced this issue Mar 19, 2020
@FrankMittelbach FrankMittelbach added this to Pool (unscheduled issues) in upcoming LaTeX2e releases via automation Mar 19, 2020
@FrankMittelbach FrankMittelbach added this to the release 2020 fall milestone Mar 19, 2020
@FrankMittelbach FrankMittelbach moved this from Pool (unscheduled issues) to Done in dev in upcoming LaTeX2e releases Mar 19, 2020
@FrankMittelbach FrankMittelbach added the fixed in dev Fixed in development branch, not in stable release label Mar 19, 2020
@FrankMittelbach
Copy link
Member

I should add that I think it is next to impossible (with reasonable effort) to detect the case that \bfdefault was changed to itself so if that happen I guess no resetting of the meta family defaults will happen.

That should now also be covered thanks to a good suggestion by @zauguin .

@schlcht
Copy link
Author

schlcht commented Mar 20, 2020

Thanks, looks good to me.

@FrankMittelbach FrankMittelbach moved this from Done in dev to In progress in upcoming LaTeX2e releases Mar 20, 2020
@FrankMittelbach FrankMittelbach removed the fixed in dev Fixed in development branch, not in stable release label Mar 20, 2020
@FrankMittelbach
Copy link
Member

FrankMittelbach commented Mar 20, 2020

For platex and friends there is an open request to support this in a more structured way:

85b982e#commitcomment-37946221

will think about how to best handle the so move back to in progress.

@FrankMittelbach FrankMittelbach moved this from In progress to Done in dev in upcoming LaTeX2e releases Apr 6, 2020
@FrankMittelbach FrankMittelbach added the fixed in dev Fixed in development branch, not in stable release label Apr 6, 2020
upcoming LaTeX2e releases automation moved this from Done in dev to Done May 28, 2020
@FrankMittelbach
Copy link
Member

Use new hook management for the hooks in fall release --- so reopen

upcoming LaTeX2e releases automation moved this from Done to In progress Aug 18, 2020
@FrankMittelbach FrankMittelbach removed the fixed in dev Fixed in development branch, not in stable release label Aug 18, 2020
@FrankMittelbach
Copy link
Member

@aminophen The NFSS commands now have hooks using the new hook management scheme --- this will become available with -dev release (III) around 1st of October (the old names should still be supported for now but if would be good to alter external code, as eventually that legacy support should get dropped).

@FrankMittelbach FrankMittelbach added the fixed in dev Fixed in development branch, not in stable release label Aug 27, 2020
@FrankMittelbach FrankMittelbach moved this from In progress to Done in dev in upcoming LaTeX2e releases Aug 27, 2020
@aminophen
Copy link
Contributor

@FrankMittelbach Thanks --- I will check the new scheme soon.

upcoming LaTeX2e releases automation moved this from Done in dev to Done Sep 9, 2020
wtsnjp added a commit to wtsnjp/platex-utf8-mirror that referenced this issue Mar 14, 2021
wtsnjp added a commit to wtsnjp/platex-utf8-mirror that referenced this issue Mar 14, 2021
@FrankMittelbach FrankMittelbach removed the fixed in dev Fixed in development branch, not in stable release label Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

3 participants