-
-
Notifications
You must be signed in to change notification settings - Fork 271
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
unknown-keyhandler disables removing from unused options #1183
Comments
It seems the cause is, compared to Lines 308 to 338 in 5064cfc
\RequirePackage{latexbug} % <--should be always the first line (see CONTRIBUTING)!
\begin{filecontents}[overwrite]{\jobname.cls}
\LoadClassWithOptions{article}
\DeclareKeys[testwork]{%
test .code = \newcommand{\foo}{#1},
bertha .code = {},
unknown .code = {}
}
\ProcessKeyOptions[testwork]
\end{filecontents}
\makeatletter
\ExplSyntaxOn
\cs_gset_protected:Npn \__keys_options_class:n #1
{
\cs_if_free:cF { @raw@opt@ \@currname . \@currext }
{
\keys_if_exist:nnTF {#1} { unknown }
{
\clist_put_right:Nv \l__keys_options_clist
{ @raw@opt@ \@currname . \@currext }
% <<< added, start
\clist_map_inline:cn { @raw@opt@ \@currname . \@currext }
{
\exp_args:NNe \clist_remove_all:Nn \@unusedoptionlist
{ \__keys_remove_equals:n {##1} }
}
% >>> added, end
}
{
\clist_map_inline:cn { @raw@opt@ \@currname . \@currext }
{
\exp_args:Ne \__keys_options_class:nnn
{ \__keys_remove_equals:n {##1} }
{##1} {#1}
}
}
}
}
\ExplSyntaxOff
\makeatother
\documentclass[a4paper,test=wtf,bertha]{\jobname}
\begin{document}
test \foo
\end{document} |
Do we feel this needs to go out as a patch level, or am I OK to fix just in dev with the intention being to aim for 2024-06-01? |
@muzimuzhi the fix doesn't look right. It basically disables the |
a) make sure we have the right fix and b) I think this can wait until 2024-06 |
Fix in hand |
@u-fischer You're right. What I proposed is simply For example, if used as
is expected, is it? But then that's different from \RequirePackage{latexbug} % <--should be always the first line (see CONTRIBUTING)!
\begin{filecontents}[overwrite]{\jobname.cls}
\LoadClassWithOptions{article}
\DeclareOption{test}{\newcommand\foo{wtf}}
\DeclareOption{bertha}{}
\DeclareOption*{}
\ProcessOptions\relax
\endinput
\end{filecontents}
\documentclass[a4paper,test,bertha,aaapaper]{\jobname}
\begin{document}
test \foo
\end{document} |
Yes I think so. Imho normally it is simply wrong if a class sets the unknown handler on the class options as there is already one: unknown keys are passed to the packages. This doesn't mean that we should not correct the issue, there may be edge case where an unknown handler can be needed, e.g. if the keys are used outside the class option list in a \SetKeys, but for the processing as class/package options such a handler should imho have no effect.
That looks wrong to me too. |
@u-fischer and @muzimuzhi: No And unknown options aren't used to set options of packages (that usage is not possible, because the unknown options list doesn't contain the full information). Packages should parse the options passed to them, and if desired by the package author also the global option list. Unfortunately there is no intermediate solution possible with the current model of Use case where the behaviour described in the first paragraph is required is simple: A class which builds upon and extends another class, without altering the option list of that class, just as done in the MWE above. And to correctly build upon another class we have to forward the options unaltered, forwarding using an |
This issue has been automatically marked as stale because it has not had recent activity. |
Brief outline of the bug
If one activates an
unknown
handler in anl3keys
-set the new option handler does no longer remove known keys from the list of unused options if used inside a class (in a package the handling seems fine).Minimal example showing the bug
Log file (required) and possibly PDF file
mwe_unknownbug.log
The text was updated successfully, but these errors were encountered: