-
-
Notifications
You must be signed in to change notification settings - Fork 274
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
Conversation
@Skillmon Could you review here? |
No rollback here: this is purely suppressing some stray warnings and there should be no sense in which people need the older behaviour. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one typo noticed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work well with my own tests with and without usage=load
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd additionally add the following test file:
\begin{filecontents*}[overwrite]{testpackage.sty}
\ProvidesExplPackage{testpackage}{2025-02-04}{}{}
\keys_define:nn { testpackage }
{
testoption .code:n = { \def \testmacro { #1 } } ,
testoption .usage:n = load
}
\ProcessKeyOptions [ testpackage ]
\endinput
\end{filecontents*}
\input{test2e}
\documentclass[testoption = false]{article}
\START
\usepackage[testoption = true]{testpackage}
\usepackage[testoption = ERROR]{testpackage}
\TYPE{\testmacro}
\END
(should type true
)
given globally (in the optional argument to \cs{documentclass}) would be | ||
repeatedly processed and could therefore lead to spurious warnings. This has |
There was a problem hiding this comment.
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.
base/testfiles/github-1619a.lvt
Outdated
\usepackage{testpackage} | ||
\usepackage{testpackage} | ||
|
||
\END |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing EOL.
Internal housekeeping
Status of pull request
Checklist of required changes before merge will be approved
\changes
entries in source includedchanges.txt
updatedltnewsX.tex
(and/orlatexchanges.tex
) updated