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

ltkeys: Global options starting surrounded by spaces in the input are not removed from the unused option list #1238

Open
Skillmon opened this issue Jan 12, 2024 · 8 comments

Comments

@Skillmon
Copy link
Contributor

Brief outline of the bug

The current code used to remove keys from the unused options list doesn't first strip spaces from the key name, which results in false-positives in the unused options list.

Minimal example showing the bug

\RequirePackage{latexbug}       % <--should be always the first line (see CONTRIBUTING)!

\begin{filecontents}{bugreport.sty}
\DeclareKeys{foo.store=\myfoo, bar.store=\mybar, baz.store=\mybaz}
\ProcessKeyOptions
\ProvidesPackage{bugreport}
\end{filecontents}

\documentclass[foo =bar, bar=baz, baz = foo]{article}

  % Any preamble code goes here
\usepackage{bugreport}

\typeout{%
  foo: \myfoo^^J%
  bar: \mybar^^J%
  baz: \mybaz^^J%
}

\begin{document}
\end{document}

Log file (required) and possibly PDF file

bugreport.log

Skillmon added a commit to Skillmon/latex2e that referenced this issue Jan 12, 2024
@Skillmon
Copy link
Contributor Author

@josephwright
Copy link
Member

Fixed in dev?

@Skillmon
Copy link
Contributor Author

It is fixed in main see #1239. The ltnews39 entry is still missing. Also, currently the fix wasn't merged into develop, so the -dev formats still throw the warning, while the normal formats don't.

@tweh
Copy link

tweh commented Jul 22, 2024

As far as I understood this issue is fixed however I came across this while implementing an expl3 class and the following MWE still show this issue (with TeXLive 2024, up-to-date on macOS, up-to-date)

\RequirePackage{latexbug}       % <--should be always the first line (see CONTRIBUTING)!

\begin{filecontents}{democlass.cls}
\ProvidesExplClass {democlass} {2024/07/22} {1} {demo}
\LoadClassWithOptions { scrbook }
\tl_new:N \l_demo_word_tl
\keys_define:nn { democlass } {
  word .tl_set:N = \l_demo_word_tl,
}
\ProcessKeyOptions [ democlass ]
\newcommand{\myword}{\tl_use:N \l_demo_word_tl}
\end{filecontents}

\documentclass[word =xx]{democlass}

% Any preamble code goes here
\usepackage{bugreport}

\begin{document}
Test: \myword
\end{document}

Am I doing something wrong or is this bug still persistent?

@muzimuzhi
Copy link
Contributor

muzimuzhi commented Jul 22, 2024

@tweh It seems the problem only reproduces with Koma-Script document classes like scrbook or scrartcl, but not with standard ones like article.

\documentclass[foo =xx]{scrbook}
\makeatletter
% article.cls, unused options: <foo>
% scrbook.cls, unused options: <foo >, note the trailing space
\PackageError{debug}{unused options: <\@unusedoptionlist>}{}
\makeatother

\begin{document}
content
\end{document}

@muzimuzhi
Copy link
Contributor

Looks like \OptionNotUsed is the culprit. Here's a reproducing example with article:

\RequirePackage{latexbug}       % <--should be always the first line (see CONTRIBUTING)!
\begin{filecontents}[force]{democlass.cls}
\LoadClass{article}
\DeclareKeys {word .code = \OptionNotUsed}
\ProcessKeyOptions
\end{filecontents}

\documentclass[word =xx]{democlass}

\begin{document}
\end{document}

In log

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

\OptionNotUsed only removes =<value> from \CurrentOption whose value may have form <key>=<value>, but doesn't trim spaces from <key>.

latex2e/base/ltclass.dtx

Lines 1522 to 1527 in 22375e7

\def\OptionNotUsed{%
\ifx\@currext\@clsextension
\xdef\@unusedoptionlist{%
\ifx\@unusedoptionlist\@empty\else\@unusedoptionlist,\fi
\expandafter\@remove@eq@value\CurrentOption=\@nil}%
\fi}

@muzimuzhi
Copy link
Contributor

One way is to add space-trimming to \@remove@eq@value, which is only used in two fully-expansion places.

\ExplSyntaxOn
% \def\@remove@eq@value#1=#2\@nil{#1} % before
\def\@remove@eq@value#1=#2\@nil{\tl_trim_spaces:n{#1}}
\ExplSyntaxOff

latex2e/base/ltclass.dtx

Lines 1524 to 1526 in 22375e7

\xdef\@unusedoptionlist{%
\ifx\@unusedoptionlist\@empty\else\@unusedoptionlist,\fi
\expandafter\@remove@eq@value\CurrentOption=\@nil}%

latex2e/base/ltclass.dtx

Lines 1744 to 1745 in 22375e7

\@expandtwoargs\@removeelement
{\expandafter\@remove@eq@value\CurrentOption=\@nil}%

@josephwright
Copy link
Member

@muzimuzhi Make a PR?

@josephwright josephwright reopened this Jul 23, 2024
muzimuzhi added a commit to muzimuzhi/latex2e that referenced this issue Jul 25, 2024
See latex3#1238.

Co-authored-by: Jonathan Spratte <jspratte@yahoo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants