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

Implement and document pgfkeys tracing. #1286

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sasozivanovic
Copy link
Contributor

Motivation for this change

In 2012, Ryan Reich wrote a handy little utility for tracing the execution of pgfkeys processing. However, his unpublished trace-pgfkeys.sty had a form of a runtime patch and stopped working upon changes in pgfkeys. In 2016, I took over maintainance of the package, by agreement with Ryan, with the idea of integrating the tracing code into pgfkeys itself. Here it is, finally. The DocStrip-like approach makes it easy to maintain, while incurring no overhead when tracing is not in effect.

While the framework integrating the tracing code into pgfkeys is of my implementation, the tracing messages (and trace levels) are mostly Ryan's, with a couple of my additions. Only the core pgfkeys.code.tex is equipped with tracing code; I'm not familiar enough with key filtering to add tracing messages to pgfkeyslibraryfiltered.code.tex.

The tracing code is based on the draft of package
`trace-pgfkeys.sty` (2012/02/06) by Ryan Reich <ryan.reich@gmail.com>.

`trace-pgfkeys` had a form of a runtime patch, and was thus difficult to
maintain.  The present implementation integrates the tracing code into
`pgfkeys.code.tex`.  The approach is DocStrip-like: every line of tracing code
occurs behind character `&`, which is set to either catcode 9 (ignore) or 14
(comment) to include or exclude the tracing code, respectively.

Consequently, the tracing code is not loaded unless specifically requested, and
therefore does not slow down normal key processing.

Signed-off-by: Sašo Živanović <saso.zivanovic@guest.arnes.si>
They somehow ended commented out.

Signed-off-by: Sašo Živanović <saso.zivanovic@guest.arnes.si>
Signed-off-by: Sašo Živanović <saso.zivanovic@guest.arnes.si>
@hmenke
Copy link
Member

hmenke commented Oct 9, 2023

https://sourceforge.net/p/pgf/patches/23/ was never migrated it seems.

@sasozivanovic
Copy link
Contributor Author

https://sourceforge.net/p/pgf/patches/23/ was never migrated it seems.

That's ok. That implementation was too complicated.

Copy link
Member

@hmenke hmenke left a comment

Choose a reason for hiding this comment

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

I appreciate the effort, but the proposed implementation is quite invasive and I'm not sure I want to have this in pgfkeys.

Would it maybe be sufficient to only expose tracing hooks inside pgfkeys macros that are then implemented by a separate pgfkeys library?

% in file pgfkeys.code.tex
\long\def\pgfkeys@use@handler#1,{%
  &\pgfkeys@tracing@hook@usehandler{#1}%
  \pgfkeys@the@handler{#1}%
  \pgfkeys@parse%
}

% in file pgfkeyslibrarytracing.code.tex
\def\pgfkeys@tracing@hook@usehandler#1{%
  \tpgfk@log@stack{Current string (first char syntax): (\detokenize{#1})}%
  \tpgfk@log@trace{First char syntax handler: \detokenize\expandafter{\pgfkeys@the@handler}}%
  \tpgfk@log@verbose{First char syntax handler code: \meaning\pgfkeys@the@handler}%
}

\catcode`\@=11
\catcode`\&=9
\let\pgfkeysloaded\undefined
\input{pgfkeys.code.tex}%
Copy link
Member

Choose a reason for hiding this comment

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

I'm not exactly happy with re-\input-ing pgfkeys.code.tex. Does this work correctly when it appears in a group or as an argument to a macro? It also requires littering pgfkeys.code.tex with

&\iffalse
stuff that doesn't survive the re-\input
&\fi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First of all, thank you for taking the time to look at the proposal.

The idea behind re-\input-ing pgfkeys.code.tex is to have zero effect on loaded pgfkeys code when tracing is not asked for. As far as I can see, this could only be achieved in two other ways, both of them inferior in my opinion:

  • Patch the macros. This was the original system. As time had proven, this is very hard to maintain, and that's why we're currently without tracing support for pgfkeys.

  • Keep two versions of the pgfkeys, say pgfkeys.code.tex and tracepgfkeys.code.tex. I see this as difficult to maintain as well.

I have tested reinput in groups before submitting, and it worked; it's even mentioned in the documentation. I have now tested input from the macros, macro arguments and command keys, and all this seems to work as well. That said, I see most users loading the tracing from the main group and outside a macro; changing the trace level is another issue, I can see that as very useful within a group, but that doesn't load anything. (Incidentally, I forgot to mention in the pull request that I didn't envision an immediate acceptance of the proposal – for changes in such a core utility, extensive testing should be done. But I guess that does without saying anyway.)

I would say that three instances of &\iffalse ... &\fi hardly count as littering. Furthermore, the stuff in there survives reinput just fine. The conditionals are there to conserve resources, i.e. to not declare another eponymous register, and in the case of \ifpgfkeys@syntax@handlers, to preserve user's setting, i.e. not forget that the user executed /handlers/first char syntax.

\ifnum\catcode`\&=9
% Here we expect that whoever set the catcode of & also set \tpgfk@ampcode.
\else
\edef\tpgfk@ampcode{\the\catcode`\&}
Copy link
Member

Choose a reason for hiding this comment

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

We “own” the \pgf@ and \pgfkeys@ prefix. It doesn't seem appropriate to also steal the \tpgfk@ prefix. Why not use \pgfkeys@trace@ or \pgfkeys@tracing@?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed.

% Usage: \tracepgfkeys[<tracing option keylist>]
% This macro loads pgfkeys.code.tex with tracing code when first executed (and
% then applies the given keylist).
\def\tracepgfkeys{\pgf@keys@utilifnextchar[{\tracepgfkeys@opt}{\tracepgfkeys@{}}}
Copy link
Member

Choose a reason for hiding this comment

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

I really dislike having extra user macros. Can't this also just use keys and values?

\pgfkeys{%
  /pgfkeys/tracing/.is if=pgfkeys@tracing@enabled,
}
\def\pgfkeys@tracing@enabledtrue{%
  % enable tracing, e.g. by re-\input pgfkeys.code.tex
}
\def\pgfkeys@tracing@enabledfalse{%
  % disable tracing
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could.

I tried to avoid this on purpuse, actually, as I thought that reinputting pgfkeys.code.tex from \pgfkeys would be frowned upon. But I'm fine with it, and the limited testing I did now shows no ill effects.

I disagree with the proposed boolean UI, though. As a user, I would expect /pgfkeys/tracing=false to annul the effect of /pgfkeys/tracing(=true), which is not the case – false does not "unload" the tracing code (unless we complicate matters even more). I would rather go for /pgfkeys/tracing[=silent|stack|trace|verbose]. In my opinion, this makes it clear(er) that some sort of tracing is active after this is issued, even if silent tracing.

% tracing code, all of which is preceded by this character, will be loaded.
% Otherwise, we set catcode of & to 14 (comment), and the tracing code will not
% be loaded.
\ifnum\catcode`\&=9
Copy link
Member

Choose a reason for hiding this comment

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

I find the whole catcode business going on here quite messy. The code becomes quite unreadable and the maintenance burden is quite high. On top of implementation logic many macros now also have tracing logic. Changing a macro now also requires changing the tracing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I appreciate the effort, but the proposed implementation is quite invasive and I'm not sure I want to have this in pgfkeys.

Respectfully, I disagree about the invasiveness. The fate of the original trace-pgfkeys shows that any non-integrated approach is too hard to maintain, and any integrated implementation will have to add something. As far as I see, the proposed implementation adds the bare minimum — tracing code sprinkled into the original macros and clearly distinguishable from non-tracing code (due to the presence of the guard &). I would call this minimally invasive.

Would it maybe be sufficient to only expose tracing hooks inside pgfkeys macros that are then implemented by a separate pgfkeys library?

Well, the hooks approach has the (one and only) advantage of being less intrusive when one is reading pgfkeys.code.tex. But in my opinion, this advantage is outweighed by the disadvantages.

  • The added layer of complexity increases the maintenance workload. In order to add or change some tracing code, one would need to match up hook names in the original and the tracing code (residing in different files).

  • It is more error-prone. It would be easy to make a mistake when matching the hook names (above); in contrast, you can't put the tracing code in a wrong place with the hook-less approach. Also, what if whoever proposes a new hook forgets to define it? At least a dummy would need to be defined.

  • It pollutes the string space (many new control sequences).

Finally, the hooks approach still does not solve the issue of the "messy catcode business", raised below. The hook invocations should still occur behind the guard (&), otherwise we are polluting the runtime content of pfgkeys macros — and not doing this, rather than the visual outlook of the code, was my primary concern for the implementation.

I find the whole catcode business going on here quite messy. The code becomes quite unreadable and the maintenance burden is quite high.

I don't understand how these two comments are related. The catcode logic in the "preamble" is one thing; the readability of the code, i.e. the pgfkeys macros, is another (and apparently a matter of taste).

On the subject of the catcodes. The approach is very similar to the familiar, tried-and-true .dtx approach. So how is this messy? (Of course, I'm willing to entertain an alternative idea, it's just that I don't have one.) Let me try to clarify the catcode logic: the bulk of pgfkeys.code.tex is read either with & having catcode comment (no tracing) or ignore (tracing); the catcode is restored at the end of the file. (I have chosen & because it was not used in pgfkeys.code.tex and because it is easily detectable in any editor with TeX syntax highlighting.)

On top of implementation logic many macros now also have tracing logic. Changing a macro now also requires changing the tracing.

Well, this is sort of the point, isn't it? (Ok, I don't know if I would call it tracing logic, but that's semantics.) Yes, if we are to have tracing integrated into pgfkeys, changing the tracing info upon changing a (core) macro would be required; or at the very least, it would be the polite thing to do. The real question is then perhaps, do we want to have integrated tracing? Do we want to have tracing at all? I have blindly assumed that this is the case, as tracing was immensely useful for me in the past (and I cannot be the only one, if Ryan took the effort of creating trace-pgfkeys in the first place), but maybe my assumption was flawed.

It's perhaps also worth pointing out that there shouldn't be much of tracing-related maintenance required. I mean, how often does the core of pgfkeys change? I have the impression that any core changes will be very limited, lest they break the ton of existing code using the utility. I looked through the changes to pgfkeys.code.tex in the past five years (the extent shown on GitHub): there was a single change which would require the addition of new tracing code. (Note that most often, new handlers and such require no tracing code, e.g. the existing tracing code shows quite nicely what's happening with .evaluate.)

One final note on maintenance. In my opinion, it is much easier for whoever develops/changes a macro to update the tracing code. They know what they are doing and why. It is an extra minute of work to add tracing. In contrast, it can take hours for an outsider (like myself) to figure out what the particular piece of code is doing, and trace it properly. And we can also get it wrong.

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.

None yet

2 participants