Fix 7701 filtered transclusion fails if "}" follows somewhere in the text

[pmario]

This PR fixes #7701 and #7797

• It contains several new tests for SIMPLE and MAXimal number of parameters.

• There are new tests specific to issue 7701

• It removes the unused code in filteredtranscludeblock.js and filteredtranscludeinline.js and changes the regexps accordingly.

• All tests pass from the CLI and in the browser

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
tiddlywiki5	✅ Ready (Inspect)	Visit Preview	Jan 31, 2024 2:40pm

[Jermolene]

Hi @pmario I think the problem here is that the regexp is too greedy. Removing the unused syntactic structures does fix it, but only because it removes that part of the regular expression. It would be better to fix it by making the regexp not be greedy.

As it happens, I do think there are possibilities for us to make use of the unused parts of the syntax that we should consider:

{{{ [tag[docs]] }}}
{{{ [tag[docs]] |variablename}}}
{{{ [tag[docs]] ||TemplateTitle}}}
{{{ [tag[docs]] |variablename||TemplateTitle}}}
{{{ [tag[docs]] }} param:value another:value}

[pmario]

As a reminder

The existing regexp does create matches for

{{{ [tag[docs]] }}}
{{{ [tag[docs]] | tooltip }}}
{{{ [tag[docs]] || TemplateTitle }}}
{{{ [tag[docs]] | tooltip || TemplateTitle }}}
{{{ [tag[docs]] }} width:40;height:50; }.class.class

The proposal is:

{{{ [tag[docs]] }}}
{{{ [tag[docs]] |variablename}}}
{{{ [tag[docs]] ||TemplateTitle}}}
{{{ [tag[docs]] |variablename||TemplateTitle}}}
{{{ [tag[docs]] }} param:value another:value}

• For readability IMO it needs to be possible to have whitespace in between the separators.
• The new pattern is different
• Since it does not have the .class.class pattern at the end it may work -> More tests will be needed.

---

I wrote: https://github.com/Jermolene/TiddlyWiki5/issues/7701#issuecomment-1772762691

@Jermolene ... The problem is caused by the trailing whitespace character after the closing braces }}} in the OP. Any other "trailing" character has the same effect. eg: {{{ [[any filter]] }}}a

The reason is, that with a trailing character the last 2 braces }} and the following text match group 4 -- The first closing brace } will be added to match group 1

• if and only if there is an other closing brace } in the text that follows.
• If there is no closing brace in the following text the whole rule fails.

---

From a pragmatic point of view a trailing character makes the whole text block an inline filtered transclusion. .. BUT since the block-rule creates some valid matches it does not switch to the inline parser.

I do have some ideas, but all of them are only partial solutions.

---

Code that causes the problem from https://github.com/Jermolene/TiddlyWiki5/issues/7701#issue-1868247366

• The trailing space after {{{ [[any filter]] }}} causes the problem
• {{{ [[any filter]] }}}x will also cause the problem

{{{ [[any filter]] }}} 

this string disappeas and the succeeding curly brace becomes wikified, or maybe it is a brace from the filtered transclusion above

}

[pmario]

@Jermolene .. with the latest commits, we do have all the parameters back. I did rename them with more generic names. So we are able to rename them again in the future, once we know, how to use them.

I did add 2 MAX tests, which test all the parameters, so we should be safe to avoid accidental bugs in the future.

[pmario]

The regexp fix for block mode was actually much simpler as expected. -- I "only" needed to look at it with some time in between.

[saqimtiaz]

If we do decide to extend the filtered transclusion syntax to support parameters, this proposed syntax {{{ [tag[docs]] |variablename||TemplateTitle}}} is discordant with the existing transclusion syntax {{MyTiddler||TemplateTitle|Parameter|SecondParameter}} in terms of the positioning of the parameters relative to the template. This would likely cause confusion amongst users. Could we adopt the existing syntax, i.e. reverse the positions of the parameters and the template without breaking backwards compatibility?

[pmario]

Could we adopt the existing syntax, i.e. reverse the positions of the parameters and the template without breaking backwards compatibility?

I'm not really sure. The regexp is super tricky. May be it is possible. Since at the moment there is only code that assigns the template to any widget-tree -> DOM ... We should have no problem with the compatibility.

At the moment only the parse-tree is populated, which should cause failing tests -- intentionally.

[twMat]

Just a note to say that I have again been affected by the bug that this PR intends to resolve. Luckily I had a vague memory of it so I could find the discussion for it, and see the workaround (to remove a trailing space). Other users would not know about this. If the PR is up to par, please merge @Jermolene

[Jermolene]

Hi @twMat @pmario as well as the bug fix this PR includes some relatively controversial new features that need further discussion before we can consider merging them.

Would it be possible to make a PR with just the bug fix?

[pmario]

TLDR; The best way would be to agree on a new proposal first. Since it is the same amount of work and doing it twice does not make too much sense.

---

The current regexp is very complex and contains some "existing but unused features".

• Completely removing them was no option
• I made a proposal, which is possible but not optimal since it is different to the transclusion-parameters as Saq found out: https://github.com/Jermolene/TiddlyWiki5/pull/7822#issuecomment-1919287164

So I will need to take that one into account and see if it is possible to come up with a new proposal, that can fix the problem and also allows us to add new features in the future without breaking everything.

As far as I can remember the }.class.class has to go, since we cannot fix the problem otherwise. I think it was the main reason for the problem as described at: https://github.com/Jermolene/TiddlyWiki5/pull/7822#issuecomment-1828062376

So: We need to agree on a new proposal first.