unknown-keyhandler disables removing from unused options

[Skillmon]

Brief outline of the bug

If one activates an unknown handler in an l3keys-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

\RequirePackage{latexbug}       % <--should be always the first line (see CONTRIBUTING)!
\begin{filecontents}[overwrite]{\jobname.cls}
\NeedsTeXFormat{LaTeX2e}
\ProvidesClass{\jobname}
\LoadClassWithOptions{article}

\DeclareKeys[testwork]{%
  test .code  = \newcommand{\foo}{#1},
  test .usage = load,
  % the following three lines give an equivalent definition of your bertha
  % option as defined with `\DeclareOption{bertha}{}`
  bertha .code = {},
  bertha .value_forbidden:n = true,
  bertha .usage = load,
  % if you use this unknown handler, you get "test" and "bertha" as unknown
  % options, if you omit it you get "a4paper" as unknown option
  unknown .code = {}
}
\ProcessKeyOptions[testwork]

\endinput
\end{filecontents}
\documentclass[a4paper,test=wtf,bertha]{\jobname}

\begin{document}
test \foo
\end{document}

Log file (required) and possibly PDF file

mwe_unknownbug.log

[muzimuzhi]

It seems the cause is, compared to \__keys_options_class:nnn, currently in \__keys_options_class:n, when unknown key exists, raw options are only appended to \l_@@_options_clist, but not removed from \@unusedoptionlist.

latex2e/base/ltkeys.dtx

Lines 308 to 338 in 5064cfc

	\cs_new_protected:Npn \@@_options_class:n #1
	{
	\cs_if_free:cF { @raw@opt@ \@currname . \@currext }
	{
	\keys_if_exist:nnTF {#1} { unknown }
	{
	\clist_put_right:Nv \l_@@_options_clist
	{ @raw@opt@ \@currname . \@currext }
	}
	{
	\clist_map_inline:cn { @raw@opt@ \@currname . \@currext }
	{
	\exp_args:Ne \@@_options_class:nnn
	{ \@@_remove_equals:n {##1} }
	{##1} {#1}
	}
	}
	}
	}
	\cs_new_protected:Npn \@@_options_class:nnn #1#2#3
	{
	\keys_if_exist:nnTF {#3} {#1}
	{
	\clist_put_right:Nn \l_@@_options_clist {#2}
	\clist_remove_all:Nn \@unusedoptionlist {#1}
	}
	{
	\clist_if_in:NnF \@unusedoptionlist {#1}
	{ \clist_put_right:Nn \@unusedoptionlist {#1} }
	}
	}

\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}

[josephwright]

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?

[u-fischer]

@muzimuzhi the fix doesn't look right. It basically disables the \@unusedoptionlist completly but one want a4paper to stay.

[FrankMittelbach]

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?

a) make sure we have the right fix and b) I think this can wait until 2024-06

[josephwright]

Fix in hand

[muzimuzhi]

@muzimuzhi the fix doesn't look right. It basically disables the \@unusedoptionlist completly but one want a4paper to stay.

@u-fischer You're right. What I proposed is simply \let\@unusedoptionlist=\@empty. So the expected behavior here is to still report unused options even when unknown key is defined?

For example, if used as \documentclass[a4paper,test=wtf,bertha,aaapaper]{\jobname}, then the warning

LaTeX Warning: Unused global option(s):
    [aaapaper].

is expected, is it?

But then that's different from \DeclareOption*{}. Example below tested on overleaf.com with pre-2022 texlive gives no "Unused global option(s)" warning.

\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}

[u-fischer]

So the expected behavior here is to still report unused options even when unknown key is defined?

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.

But then that's different from \DeclareOption*{}.

That looks wrong to me too.

[Skillmon]

@u-fischer and @muzimuzhi: No a4paper shouldn't be reported as an unknown option, since it's known to article. The effect of an unknown handler is to not add unknown options to the list, but known options should still be removed. If the author of the package wants to add to the unknown options list he should then do so inside of the unknown processor.

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 \ProcessKeyOptions, where an unknown handler is triggered and keys are still added to the unknown options list.

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. article is built upon, and gets an unaltered option list, it adds the options it doesn't know to the unused options list (because altering article's behaviour in that regard isn't possible), and we only want to remove from that list afterwards, not add to it. Hence the unknown option handler to suppress any false positives of options known to article but not to our class.

And to correctly build upon another class we have to forward the options unaltered, forwarding using an unknown handler with \PassOptionsToClass doesn't do the trick, because edge cases can't be distinguished.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity.