-
Notifications
You must be signed in to change notification settings - Fork 2
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
Remove extra evaluations and unescapings #9
base: master
Are you sure you want to change the base?
Conversation
It seems list functions that return unescaped wikitext parse it twice, so I'm gonna work on it a little more. |
Sounds good, thanks so much for your contributions already!! |
Well, thank you all for still maintaining it, and for taking the time to review these PRs. :) |
I wrote this had no impact on code, but it is not true. The base issueUsing the changes from this PR with templates from an existing wiki it caused a (subtle) change, a beneficial one in my case, but still a breaking change: generated wikitext with transcluded wikitext syntax will have it evaluated. For example, This means, this PR (as of now) is a breaking change for the The issue with unescapingThis is a side-effect we have to deal with when unescaping: after the text is unescaped, we need to parse it again, so we parse Changing it would mean variables can no longer contribute reliably to unescaping, e.g. So I'll double down on what I said last week, and not suggest to remove the extra parsing from unescaped text in this PR. |
Mistakenly rolled back some lines in previous branch merge
…uateUnescaped` function so we can use the same behaviors when parsing unescaped parameters and unescaped outputs.
This makes it harder to track where wikitext is evaluated, and consequently, most of the time it is evaluated or unescaped more times than it should be
If the frame is not a template one, there is no arguments anyway, so there is no benefit in processing unescaped wikitext within a child frame.
Various functions trim or unescape their arguments, while these were already trimmed or unescaped. Also expand patterns directly, so we don't have unexanped nodes or untrimmed strings wandering around.
Currently, ParserPower::expand() expands values, that are expanded again when arguments are retrieved by name. To deduplicate these expansions, ParserPower::expand() no longer expands values (only keys), so we could lazily evaluate some values in the future.
Variables are replaced when arguments are parsed. When iterating over a list, arguments are values/patterns are re-evaluated each iteration, while nothing was unescaped, we only replaced evaluated wikitext with evaluated wikitext, so all we have at that time is evaluated wikitext.
…TwoSetFieldPattern` Missed that function in the 2 previous commits.
Some functions make frame arguments available when replacing variables after unescaping. This leads to unexpected results, but changing it is out of scope of this PR, so for now add a `WITH_ARGS` flag to keep this behavior.
Context
Parser functions always take wikitext arguments. Unlike tags, I would expect some behaviors to always apply to the arguments passed, whatever the function does in practice:
If the function modifies an argument, it would do it after having normalized the argument. Consequently, if we want to pass wikitext that would still look the same once normalized, we can freeze the wikitext (e.g. by using
{{(}}
or{{!}}
) or use<nowiki/>
tags.Some ParserPower functions apply an additional normalization step:
Then, once all argument modifications have been applied, the function may re-evaluate variables within the unescaped wikitext.
This provides another way to circumvent normalization: we can use escape sequences within the wikitext to prevent variable syntax from being recognized, and spaces from being removed.
An example of the issue
Let
Template:Quote
be a template that would print its 1st argument within quotes. We do not want spaces inside the quotes, so we would basically want to use:Suppose we want to pass
{{!}}
as argument, but we want it to be printed as-is. From the solutions introduced above, we could:<nowiki/>
tags, e.g.:Outputs can be found here. While both the 2nd and 3rd approaches yield
"{{!}}"
, the 1st one yields"|"
. This is because the#trim
function evaluates variables within its argument (by expanding the given pre-processor node), then trims spaces, and finally returns it while telling the parser to evaluate variables (by setting the'noparse'=false
flag).The issue
Various parser functions and tags in ParserPower evaluate variables within wikitext twice without having unescaped anything between, or unescape wikitext twice.
While changing it may not particularly allow us to do more things than we can already do, each variable evaluation that a parser function tries to do generates and evaluates additional pre-processor nodes. This takes some small additional parsing time, and artificially makes parser reports larger than they should.
Proposed changes
Remove extra variable evaluation, trimming, and unescaping steps. More precisely:
To achieve this, a few unrelated changes have been made:
Below is a list of all variable evaluation/unescaping steps that were removed when the wikitext may have contained frozen variable syntax or wikitext escaped multiple times (that was previously evaluated/unescaped twice and is no longer in this PR), i.e. all potentially breaking changes:
{{#trim:}}
{{#or:}}
{{#follow:}}
{{#listfilter:}}
list
anddefault
arguments (evaluated twice).pattern
argument (evaluated thrice if the list is not empty).{{#listmap:}}
list
anddefault
arguments (evaluated twice).pattern
argument (evaluated thrice if the list is not empty).{{#lstmap:}}
{{#listunique:}}
list
anddefault
arguments (evaluated twice).pattern
argument (evaluated thrice if the list is not empty).{{#listsort:}}
list
anddefault
arguments (evaluated twice).pattern
argument (evaluated thrice if the list is not empty).{{#listmerge:}}
list
anddefault
arguments (evaluated twice).matchpattern
andmergepattern
arguments (evaluated thrice if the list is not empty).