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

Process global options once per package #1625

Merged
merged 4 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions base/changes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ to completeness or accuracy and it contains some references to files that are
not part of the distribution.
================================================================================

2025-01-15 Joseph Wright <[email protected]>
* ltkeys.dtx
Parse global options only once per package (gh/1619)

2025-01-11 Udi Fogiel <[email protected]>

* ltluatex.dtx (subsubsection{Handlers}):
Expand Down
11 changes: 11 additions & 0 deletions base/doc/ltnews41.tex
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,17 @@ \subsection{Prevent \texttt{cmd} hook from defining an undefined command}
%
\githubissue{1591}

\subsection{Process global options once per package}

In 2022, we introduced key--value (keyval) option processing in the
kernel~\cite{41:ltnews35}. This also added the idea that keys could have scope:
load-only, preamble-only and general use. However, we overlooked that an option
given globally (in the optional argument to \cs{documentclass}) would be
repeatedly processed and could therefore lead to spurious warnings. This has
Comment on lines +413 to +414
Copy link
Contributor

Choose a reason for hiding this comment

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

The, imho, much bigger issue than these spurious warnings is that load options are applied just once while every other option is applied multiple times (potentially overwriting any explicitly set option for this package). But otherwise the wording is fine. This is just a note, not a request for change.

now been corrected: global options are seen exactly once per package by the
keyval-based option handling system.
%
\githubissue{1619}

%\section{Changes to packages in the \pkg{amsmath} category}

Expand Down
17 changes: 12 additions & 5 deletions base/ltkeys.dtx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
%<*driver>
% \fi
\ProvidesFile{ltkeys.dtx}
[2024/06/20 v1.0o LaTeX Kernel (Keyval options)]
[2025/01/15 v1.0p LaTeX Kernel (Keyval options)]
% \iffalse
\documentclass{l3doc}
\GetFileInfo{ltkeys.dtx}
Expand Down Expand Up @@ -277,6 +277,7 @@
% \changes{v1.0l}{2022/10/20}
% {Define key option handler in \pkg{ltkeys}}
% \changes{v1.0i}{2022/07/05}{Support \cs{CurrentOption}}
% \changes{v1.0p}{2025/01/15}{Only process global options on first pass}
% \begin{macro}{\@@_options_end:}
% The main function calls functions to collect up the global and local
% options into \cs{l_@@_options_clist} before calling the
Expand All @@ -286,17 +287,23 @@
% current family, and is cleaned up afterwards if required. To allow
% the \LaTeXe{} layer to know this mechanism is active, and to deal
% with the key family not matching the file name, we store the family
% in all cases.
% in all cases. Global options are only considered the first time a
% package is loaded: this is tracked using
% |opt@handler@\@currname.\@currext|, as this is defined once keyval
% processing has been applied for the first time.
% \begin{macrocode}
\cs_new_protected:Npn \@@_options:n #1
{ \@@_options_expand_module:Nn \@@_options_aux:n {#1} }
\cs_new_protected:Npn \@@_options_aux:n #1
{
\cs_gset_protected:cpn { opt@handler@\@currname.\@currext }
{ \ProcessKeyOptions [ #1 ] }
\cs_set_protected:Npn \@@_option_end: { }
\clist_clear:N \l_@@_options_clist
\@@_options_global:n {#1}
\cs_if_exist:cF { opt@handler@ \@currname . \@currext }
{
\@@_options_global:n {#1}
\cs_gset_protected:cpn { opt@handler@ \@currname . \@currext }
{ \ProcessKeyOptions [ #1 ] }
}
\@@_options_local:
\keys_if_exist:nnF {#1} { unknown }
{
Expand Down
21 changes: 21 additions & 0 deletions base/testfiles/github-1619a.lvt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
\begin{filecontents*}[overwrite]{testpackage.sty}
\ProvidesExplPackage{testpackage}{2025-02-04}{}{}
\keys_define:nn { testpackage }
{
testoption .code:n = { \def \testmacro { seen } } ,
testoption .usage:n = load
}
\ProcessKeyOptions [ testpackage ]
\endinput
\end{filecontents*}

\input{test2e}

\documentclass[testoption]{article}

\START

\usepackage{testpackage}
\usepackage{testpackage}

\END
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing EOL.

5 changes: 5 additions & 0 deletions base/testfiles/github-1619a.tlg
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
This is a generated file for the LaTeX2e validation system.
Don't change this file in any respect.
(testpackage.sty
Package: testpackage ....-..-..
)
21 changes: 21 additions & 0 deletions base/testfiles/github-1619b.lvt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
\begin{filecontents*}[overwrite]{testpackage.sty}
\ProvidesExplPackage{testpackage}{2025-02-04}{}{}
\keys_define:nn { testpackage }
{
testoption .code:n = { \def \testmacro { seen } } ,
testoption .usage:n = load
}
\ProcessKeyOptions [ testpackage ]
\endinput
\end{filecontents*}

\input{test2e}

\documentclass[testoption]{article}

\START

\usepackage[testoption = false]{testpackage}
\usepackage[testoption = true]{testpackage}

\END
josephwright marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 6 additions & 0 deletions base/testfiles/github-1619b.tlg
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
This is a generated file for the LaTeX2e validation system.
Don't change this file in any respect.
(testpackage.sty
Package: testpackage ....-..-..
)
LaTeX Warning: Package "testpackage" has already been loaded: ignoring load-time option "testoption".
23 changes: 23 additions & 0 deletions base/testfiles/github-1619c.lvt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
\begin{filecontents*}[overwrite]{testpackage.sty}
\ProvidesExplPackage{testpackage}{2025-02-04}{}{}
\keys_define:nn { testpackage }
{
testoption .code:n = { \def \testmacro { seen } } ,
testoption .usage:n = load
}
\ProcessKeyOptions [ testpackage ]
\endinput
\end{filecontents*}

\input{test2e}

\documentclass[testoption = true]{article}

\START

\usepackage{testpackage}
\usepackage{testpackage}
\usepackage[testoption = true]{testpackage}
\usepackage[testoption = false]{testpackage}

\END
josephwright marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 7 additions & 0 deletions base/testfiles/github-1619c.tlg
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
This is a generated file for the LaTeX2e validation system.
Don't change this file in any respect.
(testpackage.sty
Package: testpackage ....-..-..
)
LaTeX Warning: Package "testpackage" has already been loaded: ignoring load-time option "testoption".
LaTeX Warning: Package "testpackage" has already been loaded: ignoring load-time option "testoption".
23 changes: 23 additions & 0 deletions base/testfiles/github-1619d.lvt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
\begin{filecontents*}[overwrite]{testpackage.sty}
\ProvidesExplPackage{testpackage}{2025-02-04}{}{}
\keys_define:nn { testpackage }
{
testoption .code:n = { \def \testmacro { seen } } ,
testoption .usage:n = load
}
\ProcessKeyOptions [ testpackage ]
\endinput
\end{filecontents*}

\input{test2e}

\documentclass[testoption = false]{article}

\START

\usepackage{testpackage}
\usepackage{testpackage}
\usepackage[testoption = true]{testpackage}
\usepackage[testoption = false]{testpackage}

\END
josephwright marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 7 additions & 0 deletions base/testfiles/github-1619d.tlg
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
This is a generated file for the LaTeX2e validation system.
Don't change this file in any respect.
(testpackage.sty
Package: testpackage ....-..-..
)
LaTeX Warning: Package "testpackage" has already been loaded: ignoring load-time option "testoption".
LaTeX Warning: Package "testpackage" has already been loaded: ignoring load-time option "testoption".
Loading