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

fix(decorations): expand pgfkeys internals in error msg #1083

Merged
merged 1 commit into from
Dec 8, 2021

Conversation

muzimuzhi
Copy link
Member

Motivation for this change

Implementation note:
Since \pgfkeyscurrentname would already been fully expanded in testers \pgfifdecoration and \pgfifmetadecoration, fully expansion, compared to one-step expansion, is also safe here.

Fixes #1082

Checklist

Please signoff your commits to explicitly state your agreement to the Developer Certificate of Origin. If that is not possible you may check the boxes below instead:

@muzimuzhi muzimuzhi requested a review from hmenke December 3, 2021 22:20
@muzimuzhi
Copy link
Member Author

Please don't merge. I need check a thing. \pgfexpl@exp@args@@Ne seems not sufficient here.

@muzimuzhi
Copy link
Member Author

muzimuzhi commented Dec 7, 2021

Please don't merge. I need check a thing. \pgfexpl@exp@args@@Ne seems not sufficient here.

Unfortunately, my last commit 7943d07 is not a solution.

Here we face a usage \pgfkeys{key={arg1}{arg2}} and what's needed is to fully expand arg1 and leave arg2 unchanged before \pgfkeys expands. Hence

  • \pgfexpl@exp@args@@Ne\pgfkeys{key={arg1}{arg2}} is wrong because it also expands arg2.
  • All the existing /.expand xxx handlers can't help because they all treat keys as a single one arg.

To continue with \pgfexpl@exp@args@@Ne / \exp_args:Ne, arg2 should be wrapped by an \unexpanded:

\pgfexpl@exp@args@@Ne\pgfkeys{key={arg1}{\unexpanded{arg2}}}

Edit: Or, redefine /errors/unknown key and the like to always expand the arg representing "current key". Cons: may lead to less helpful error messages.

@ilayn
Copy link
Member

ilayn commented Dec 7, 2021

Put it into another single key say errors/unknown key expanded that has /.code/expand once={{/pgf/decoration/\pgfkeyscurrentname}{#####1}} or whichever number is needed such that error message can be controlled. since nobody is using this there will be no backwards compatibility issue.

Comment on lines 20 to 23
% fully expand \pgfkeyscurrentname before being passed to
% `/errors/unknown key`
{\pgfexpl@exp@args@@Ne\pgfkeys{/errors/unknown
key={/pgf/decoration/\pgfkeyscurrentname}{\unexpanded{#1}}}}},%
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we need \unexpanded anyway we are better off using the more idiomatic /.expanded.

Suggested change
% fully expand \pgfkeyscurrentname before being passed to
% `/errors/unknown key`
{\pgfexpl@exp@args@@Ne\pgfkeys{/errors/unknown
key={/pgf/decoration/\pgfkeyscurrentname}{\unexpanded{#1}}}}},%
{\pgfkeys{/errors/unknown
key/.expanded={/pgf/decoration/\pgfkeyscurrentname}{\unexpanded{#1}}}}},%

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, you're right. Should I remove pgfutil-expl.tex?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please. We can do that later in a more coordinated fashion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunatly,

  • \pgfexpl@exp@args@@Ne\pgfkeys{key={... \pgfkeyscurrentname ...}} and
  • \pgfkeys{key/.expanded={... \pgfkeyscurrentname ...}}

are different. In the first case, value of \pgfkeyscurrentname before this \pgfkeys is used. While in the second case, value of \pgfkeyscurrentname is updated to always hold .expanded, the name of just parsed handler.

This change caused the PR failed to fix the original issue, see also #1082 (comment).

Fully expand `\pgfkeyscurrentname` before being used in first-arg of
`/errors/unknown key`.

Co-authored-by: Henri Menke <henri@henrimenke.de>
Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
@hmenke hmenke merged commit 1cba64a into pgf-tikz:master Dec 8, 2021
@muzimuzhi muzimuzhi deleted the deco-unknown-key branch December 8, 2021 13:11
muzimuzhi added a commit to muzimuzhi/pgf that referenced this pull request Jan 10, 2022
Fixed a raw use of `\unexpanded` introduced in pgf-tikz#1083, commit
  cb21347

Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
muzimuzhi added a commit to muzimuzhi/pgf that referenced this pull request Dec 5, 2024
Introduced by previous fix pgf-tikz#1083.

Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
muzimuzhi added a commit to muzimuzhi/pgf that referenced this pull request Dec 5, 2024
Introduced by previous fix pgf-tikz#1083.

Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
muzimuzhi added a commit to muzimuzhi/pgf that referenced this pull request Dec 5, 2024
Introduced by previous fix pgf-tikz#1083.

Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
muzimuzhi added a commit to muzimuzhi/pgf that referenced this pull request Dec 5, 2024
Introduced by previous fix pgf-tikz#1083.

Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
muzimuzhi added a commit to muzimuzhi/pgf that referenced this pull request Dec 5, 2024
Introduced by previous fix pgf-tikz#1083.

Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
muzimuzhi added a commit to muzimuzhi/pgf that referenced this pull request Dec 5, 2024
Introduced by previous fix pgf-tikz#1083.

Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

\pgfkeyscurrentname shown in unknown key error messages
4 participants