Fix foreach not remembering nodes between iterations #1304
Conversation
|
(Still a bit new to contributing to a Big Project ...) The latest commit adds what I think is a more flexible approach to saving and restoring these macros and dimensions, which I've then extended to include My naming convention might not quite fit with the PGF ethos ... happy to change if so. |
| @@ -2580,28 +2580,64 @@ | |||
|
|
|||
| \def\tikz@fchar oreach{\tikz@foreach}% | |||
|
|
|||
| \def\tikz@foreach{% | |||
| \def\tikzforeach@smugglers@cove{} | |||
| \let\tikztostart=\relax | |||
There was a problem hiding this comment.
Maybe move this to tex/generic/pgf/frontendlayer/tikz/libraries/tikzlibrarytopaths.code.tex where \tikztostart (and \tikztotarget) originally come from.
There was a problem hiding this comment.
I had a look at tikzlibrarytopaths.code.tex and while that is where \tikztostart is used, it is never defined there. The definitions are contained in the core TikZ parser in that whenever TikZ encounters a coordinate then it sets \tikztostart to something sensible so that if the next command is a to path then the start is set up correctly. Similarly with \tikz@tangent (which is mainly used in the calc library).
So I'm happy to move it, but just want to double-check first.
| } | ||
|
|
||
|
|
||
| \def\tikz@patched@foreach{% |
There was a problem hiding this comment.
I guess you renamed this for testing purposes, but it needs to be renamed back to \tikz@foreach.
| \def\tikzforeach@smuggle@macro#1{% | ||
| \pgfutil@ifx#1\relax{}{% | ||
| \expandafter | ||
| \pgfutil@g@addto@macro | ||
| \expandafter | ||
| \tikzforeach@smugglers@cove | ||
| \expandafter | ||
| {% | ||
| \expandafter\def\expandafter#1\expandafter{#1}% | ||
| }% | ||
| }% | ||
| } | ||
|
|
||
| \def\tikzforeach@smuggle@dimen#1{% | ||
| \pgfutil@ifx#1\relax{}{% | ||
| \expandafter | ||
| \pgfutil@g@addto@macro | ||
| \expandafter | ||
| \tikzforeach@smugglers@cove | ||
| \expandafter | ||
| {% | ||
| \expandafter#1\expandafter=\the#1\relax% | ||
| }% | ||
| }% | ||
| } | ||
|
|
||
| \def\tikzforeach@smugglers@loot{% | ||
| % | ||
| \tikzforeach@smuggle@macro\tikz@moveto@waiting% | ||
| \tikzforeach@smuggle@macro\tikztostart% | ||
| \tikzforeach@smuggle@macro\tikz@tangent% | ||
| % | ||
| \tikzforeach@smuggle@dimen\tikz@lastx% | ||
| \tikzforeach@smuggle@dimen\tikz@lasty% | ||
| \tikzforeach@smuggle@dimen\tikz@lastxsaved% | ||
| \tikzforeach@smuggle@dimen\tikz@lastysaved% | ||
| % | ||
| } |
There was a problem hiding this comment.
I like this approach (and the naming scheme 😁), but I think it would be even better if we could make this more general using pgfkeys, so that a user can register their own loot.
There was a problem hiding this comment.
Are you happy with just remember macro and remember dimension for the keys? I've not found any other keys in the tikz namespace with those names.
|
That sounds do-able, I would be inclined to keep the current methods and simply wrap them in keys, then the lower-level methods are available to library authors. I'll also implement the other things you pointed out. |
Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
Fixes pgf-tikz#1193 Co-authored-by: Leah Neukirchen <leah@vuxu.org>
to avoid a subtle constraint of `/.add` that
/.add={<arg1>}{<arg2>} % and
/.add={<arg1>} {<arg2>}
are different and the second is, in most cases, wrong usage.
Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
* docs: fix typos Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
Fix typo in pgfkeys manual
Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
Co-authored-by: Qrrbrbirlbel <qrrbrbirlbel+github@gmail.com>
Caused by luaotfload v3.24, the first "bad" commit is
latex3/luaotfload@ca49685
Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
doc: https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/adding-a-workflow-status-badge Signed-off-by: muzimuzhi <muzimuzhi@gmail.com>
Bumps [actions/github-script](https://github.com/actions/github-script) from 6 to 7. - [Release notes](https://github.com/actions/github-script/releases) - [Commits](actions/github-script@v6...v7) --- updated-dependencies: - dependency-name: actions/github-script dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Toni Dietze <Flupp@users.noreply.github.com>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 3 to 4. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v3...v4) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
`maxprintline` defaults to 9999 since l3build 2023-03-22. https://github.com/latex3/l3build/blob/main/CHANGELOG.md#2023-03-22 Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
- Replace deprecated `ifluatex` package with `iftex` - Drop setting input encoding to `utf8` `utf8` has been the new default since LaTeX2e release 2018-04-01. - Drop setting `T1` font encoding for `dvisvgm` `dvisvgm` has supported OpenType fonts for years. Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
and upload a second artifact containing pdf and all aux files Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
In the code:
```
\draw (A) foreach \nd in {B,C} { -- (\nd) };
```
then the line to `C` is drawn from `A`, not `B`. This is because the
foreach loop does not update the macro that remembers that the last
coordinate was a node. This fixes that issue.
See pgf-tikz#1303
Signed-off-by: Andrew Stacey <loopspace@mathforge.org>
Signed-off-by: Andrew Stacey <loopspace@mathforge.org>
This adds in the saving code for `\tikztostart` and `\tikz@tangent`, plus provides a more robust and maintainable method of saving the various macros and dimensions by creating a single (global) token list that will restore them to their given values upon invocation. Signed-off-by: Andrew Stacey <loopspace@mathforge.org>
Co-authored-by: Rocky Zhang <rockyzhz@gmail.com>
Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
The functionality of customized l3build target (`target_list` entry) `revisionfile` has been replaced with `l3build tag` in 9efb7e4 (refactor(ci)!: use l3build tagfiles, 2021-12-15), and the definition of `revisionfile()` has been removed in the same commit. Signed-off-by: Yukai Chou <muzimuzhi@gmail.com>
Fix in context -- "question" should be singular:
<quote>
I wish to start with the question “What is TikZ?”
</quote>
Signed-off-by: Alan D. Salewski <ads@salewski.email>
…1310) Fix in context (in section "Repeating Things: The Foreach Statement"): <quote> Normally, when a list item '...' is encountered, there should already have been /two/ list items before it, which were numbers. </quote> Signed-off-by: Alan D. Salewski <ads@salewski.email>
The keys are `remember macro` and `remember dimension`. Also `\tikz@foreach` was erroneously called `\tikz@patched@foreach` (copied from my original testing file). Signed-off-by: Andrew Stacey <loopspace@mathforge.org>
|
In trying to retrospectively sign my commits, I think I messed up the version history. I'll create a fresh branch of the current pgf repo and re-implement my patch, then resubmit it. |
Or you can force push to |
I did that, but then it's listing a whole slew of commits from the original as if they were fresh commits on this branch. Maybe that's not a problem and that when it's merged into the main branch then those will all get recognised as being the same commits as the original ones. I just don't know! Well, let me say that I'm quite happy to create a fresh branch and add in my changes, signing them properly this time - if it's needed. But I guess the most important thing first is to make sure the code is right and worry about the version structure afterwards. |
Err GitHub thinks all commits merged by loopspace@f2fa2e4 were authored by original authors but committed by you. Not sure if that's normal behavior of a merge commit. |
|
I've made a new branch with the code changes where I've signed-off my commit from the outset. I'll close this pull request and open a new one from that branch, hopefully that won't have all the weird merges. |
|
New pull request: #1316 |
Motivation for this change
Fixes #1303 by remembering the last node between iterations of a foreach loop on a path.
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: