Skip to content

some fixes for old version of tikz, such as TeXLive <= 2018#22

Open
tdwiser wants to merge 3 commits into
mcnees:masterfrom
tdwiser:fix-old-tikz
Open

some fixes for old version of tikz, such as TeXLive <= 2018#22
tdwiser wants to merge 3 commits into
mcnees:masterfrom
tdwiser:fix-old-tikz

Conversation

@tdwiser

@tdwiser tdwiser commented Mar 26, 2021

Copy link
Copy Markdown

Addresses Issue #21 (not yet extensively tested on current TeXLive!!!)

Comment thread gridpapers.dtx Outdated
\RequirePackage{xcolor}
\RequirePackage{tikz}
\usetikzlibrary{patterns.meta,calc}
\usetikzlibrary{calc}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see that \usetikzlibrary{patterns.meta} is on line 774 below. Why is that necessary?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The old version of patterns.meta (~2015, which is what I have in TeXLive 2018) redefines some tikz-internal commands (specifically \pgf@declarepattern) to use macros that pgf doesn't know about. So the patterns other than lightcones which use pgfdeclarepatternformonly break.

Comment thread gridpapers.dtx Outdated
}
\tikzdeclarepattern{
name=lightcones,
\tikzdeclarepattern{name=lightcones,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This linebreak removal was not needed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I just looked at your issue #21 and saw your comment about whitespace sensitivity. Is this what you were referring to?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes; \tikzdeclarepattern can't handle whitespace before name= on old versions. I get "pattern lightcones not defined" (paraphrased) if I include the linebreak.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That is very surprising... that makes it sound like it's doing some low-level parsing. Anyway, does it work if you instead add a percent line ending?

\tikzdeclarepattern{%
  name=lightcones,

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, it seems the linebreak is the problem, not all whitespace. I can push a commit changing this if you like!

@tdwiser tdwiser Mar 26, 2021

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

TeXLive 2018 patterns.meta code snippet:
\def\pgfdeclarepattern#1{% \begingroup% \nullfont% \def\pgf@pat@options{#1}% \pgfkeys{/pgf/patterns/.cd, #1}% \pgf@declarepattern% \endgroup% } \def\pgf@declarepattern{% \pgfifpatternundefined{\pgf@pat@name}{%

Latest patterns.meta code snippet:
\def\pgfdeclarepattern#1{% \begingroup% \nullfont% \def\pgf@pat@options{,#1}%
...
\pgf@declarepattern@meta% \endgroup% }% \def\pgf@declarepattern@meta{%
...

In the new version, a comma has been added (fixing the linebreak issue, I suspect) and the helper command is renamed with meta at the end to avoid overwriting the old pgf command. So both these were bugs in PGF/Tikz that have been fixed only (relatively) recently.

Comment thread gridpapers.dtx
Comment thread gridpapers.dtx
Comment on lines -773 to +776
\GP@declarelightconepat
\GP@declaredotpat

\usetikzlibrary{patterns.meta}
\GP@declarelightconepat

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this needed? Why not keep the \usetikzlibrary{patterns.meta} up above (like it was originally), and keep the order of pattern declaration the same?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

See comment above on where it was removed. The 'early' GP@declares use pgfdeclarepatternformonly; GP@declarelightconepat uses tikzdeclarepattern, which is part of patterns.meta. This is a kludgy workaround for the fact that old versions of patterns.meta break pgf's pattern declaration.

@duetosymmetry

Copy link
Copy Markdown
Collaborator

Thanks for your help, @tdwiser! Please see questions above.

@tdwiser tdwiser requested a review from duetosymmetry March 26, 2021 22:28
@tdwiser

tdwiser commented Mar 26, 2021

Copy link
Copy Markdown
Author

I added some short explanations in comments to hopefully preserve these bugfixes in future versions!

@duetosymmetry

Copy link
Copy Markdown
Collaborator

@mcnees and @tdwiser, should we make this PR dependent on whether or not we can fix the pattern shifting bug? We separately talked about fixing it for light cones by no longer using the pgf/tikz patterns, and just drawing a tilted grid (#23). If we completely avoid using pgf/tikz patterns, then the bugs related to patterns.meta go away (the dashing one is independent). But can we completely avoid the pattern code, or do we even want to? Maybe the pgf folks will figure out the correct shifting?

@tdwiser

tdwiser commented Mar 29, 2021

Copy link
Copy Markdown
Author

One option to clean up this PR and make it less dependent on the lightcone/pattern issue would be translating all patterns to the new tikz format, which is what @mcnees did for some of them already in #19 . The only thing to ensure there is that we protect that first newline with a %. If you like this idea I can work on implementing it and testing on TL2018.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants