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

When using \KeyValue, lttemplates and xtemplate have different results #1486

Open
Sophanatprime opened this issue Oct 7, 2024 · 9 comments · Fixed by #1487
Open

When using \KeyValue, lttemplates and xtemplate have different results #1486

Sophanatprime opened this issue Oct 7, 2024 · 9 comments · Fixed by #1487
Assignees
Labels
bug category base (latex) fixed in dev Fixed in development branch, not in stable release

Comments

@Sophanatprime
Copy link

Sophanatprime commented Oct 7, 2024

Brief outline of the bug

When using \KeyValue, lttemplates and xtemplate have different results. lttemplates may cause endless loop.

Minimal example showing the bug

\RequirePackage{latexbug}
\documentclass{article}

\ifdefined\NewTemplateType
  \NewTemplateType{foo}{0}    % current LaTeX version
\else
  \usepackage{xtemplate}      % TeXLive 2023
  \DeclareObjectType{foo}{0}
\fi

\def\foocode{}
\DeclareTemplateInterface{foo}{FOO}{0}
  {
    foo.code : tokenlist ,
    code.foo : tokenlist = \KeyValue{foo.code} ,
  }
\DeclareTemplateCode{foo}{FOO}{0}
  {
    foo.code = \foocode ,
    code.foo = \foocode ,
  }
  {\AssignTemplateKeys (\foocode)}

\DeclareInstance{foo}{i1}{FOO}{ foo.code=A }
\DeclareInstance{foo}{i2}{FOO}{ code.foo=B }

\begin{document}

\UseInstance{foo}{i1}

\UseInstance{foo}{i2}

\end{document}

Overleaf with TeXLive 2023 works well, but on my computer, there is an endless loop.

It seems there is no \__xtemplate_assign_tokenlist: in lttemplates
https://github.com/latex3/latex3/blob/1a856471a905fe70b564ff645f6f390bd2a019af/l3packages/xtemplate/xtemplate-2023-10-10.sty#L971-L995

I simply put them in the preamble replaced xtemplate with template and changed \__xtemplate_if_key_value:oTF to \__template_if_key_value:VTF, it works as in TeXLive 2023.

Log file (required) and possibly PDF file

demo.log

@josephwright josephwright self-assigned this Oct 7, 2024
@josephwright
Copy link
Member

Ah, I see what's happened - I streamlined the auxiliaries a bit, but I'd not allowed for the fact that there needs to be different handling for token lists and registers in terms for \KeyValue. A fix will be in later today in dev for the 2024-11-01 release.

@muzimuzhi
Copy link
Contributor

947d414 (Re-factor internals for template var setting, 2024-04-15) merged various \__template_assign_<type>: to a centric \__template_assign_variable:. But for commalist/clist and tokenlist/tl types, a V-type expansion was lost when a template value starts with \KeyValue.

Try this

\RequirePackage{latexbug}
\documentclass{article}

\ExplSyntaxOn
\cs_undefine:N \__template_assign_variable:
\cs_new_protected:Npn \__template_assign_variable:
  {
    \exp_args:Nee \__template_assign_variable:nn
      { \__template_map_var_type: }
      { \bool_if:NT \l__template_global_bool { g } }
  }

\cs_new_protected:Npn \__template_assign_variable:nn #1#2
  {
    \str_case_e:nnTF { #1 }
      {
        { tl } {}
        { clist } {}
      }
      % tl and clist need :NV if \l__template_value_tl starts with \KeyValue
      % see https://github.com/latex3/latex2e/commit/947d41437824b2a69ef0a3644128c574ae6f72df
      { \__template_assign_variable:cc { #1 _ #2 set:NV } { #1 _ #2 set:Nn } }
      { \__template_assign_variable:cc { #1 _ #2 set:Nn } { #1 _ #2 set:Nn } }
  }

\cs_new_protected:Npn \__template_assign_variable:NN #1#2
  {
    \__template_if_key_value:VTF \l__template_value_tl
      {
        \__template_key_to_value:
        \tl_put_right:Ne \l__template_assignments_tl
          {
            #1 \exp_not:V \l__template_var_tl
             { \exp_not:V \l__template_value_tl }
          }
      }
      {
        \tl_put_right:Ne \l__template_assignments_tl
          {
            #2 \exp_not:V \l__template_var_tl
             { \exp_not:V \l__template_value_tl }
          }
      }
    \tl_show:N \l__template_assignments_tl
  }
\cs_generate_variant:Nn \__template_assign_variable:NN {cc}
\ExplSyntaxOff
\ExplSyntaxOff

\ifdefined\NewTemplateType
  \NewTemplateType{foo}{0}    % current LaTeX version
\else
  \usepackage{xtemplate}      % TeXLive 2023
  \DeclareObjectType{foo}{0}
\fi

\def\foocode{}
\DeclareTemplateInterface{foo}{FOO}{0}
  {
    foo.code : tokenlist ,
    code.foo : tokenlist = \KeyValue{foo.code} ,
  }
\DeclareTemplateCode{foo}{FOO}{0}
  {
    foo.code = \foocode ,
    code.foo = \foocode ,
  }
  {\AssignTemplateKeys (\foocode)}

\DeclareInstance{foo}{i1}{FOO}{ foo.code=A }
\DeclareInstance{foo}{i2}{FOO}{ code.foo=B }

\begin{document}

\UseInstance{foo}{i1} % typesets "(A)"

\UseInstance{foo}{i2} % typesets "(B)"

\end{document}

@josephwright
Copy link
Member

We also need to cover fp: I think might be best to use a low-level test here (\token_if_macro:NTF), as this really is about how things are implemented.

@josephwright
Copy link
Member

Also, there are some stray braces (likely going back a long way - perhaps we had o-type expansion at one point). @muzimuzhi do you want to make a PR or are you happy to leave to me to fix?

@muzimuzhi
Copy link
Contributor

As I'm far from familiar with lttemplate, I'm happy to leave this to you :).

@muzimuzhi
Copy link
Contributor

muzimuzhi commented Oct 7, 2024

We also need to cover fp

Not necessary, as \fp_set:Nn \l_tmpa_fp {\l_tmpa_fp } is a valid assignment. But as \token_if_macro:NTF is faster than \str_case_e:nnTF, expanding fp once does no harm too.

BTW the test needed here is similar to the logic in \__exp_eval_register:N except for the \tex_the:D for registers.

@josephwright
Copy link
Member

We also need to cover fp

Not necessary, as \fp_set:Nn \l_tmpa_fp {\l_tmpa_fp } is a valid assignment. But as \token_if_macro:NTF is faster than \str_case_e:nnTF, expanding fp once does no harm too.

True but I think we might be best off simply making a clear distinction. Anyway, do you want to do it, or should I?

@muzimuzhi
Copy link
Contributor

As I'm far from familiar with lttemplate, I'm happy to leave this to you :).

@josephwright
Copy link
Member

I have a shorter approach I'll go with: PR very shortly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug category base (latex) fixed in dev Fixed in development branch, not in stable release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants