TiddlyWiki / TiddlyWiki5

A self-contained JavaScript wiki for the browser, Node.js, AWS Lambda etc.
https://tiddlywiki.com/
Other
7.97k stars 1.18k forks source link

[BUG] 5.3.1 $macrocall parse/render of $button reveals core \parameters code #7706

Closed CodaCodr closed 1 year ago

CodaCodr commented 1 year ago

Describe the bug

Calling <<my-macro {{$:/core/images/down-arrow}}>> works fine.

Calling <$macrocall $name=my-macro btn-text={{$:/core/images/down-arrow}} /> prints...

\parameters (size:"22pt")

Expected behavior

No response

To Reproduce

See screenshot, tested on tiddlywiki.com

Screenshots

image

TiddlyWiki Configuration

Additional context

No response

CodaCodr commented 1 year ago

Workaround,

<!-- 7706 workaround add quotes around the transclude -->
<$macrocall $name=my-btn btn-text="{{$:/core/images/down-arrow}}"/>
pmario commented 1 year ago

See: https://github.com/Jermolene/TiddlyWiki5/issues/7580#issuecomment-1630458075

pmario commented 1 year ago
\define my-btn(btn-text)
<$button>$btn-text$</button>
\end

There are 2 things in this example, that should not be used anymore.

$btn-text$ ... This type of text-substitution should not be used anymore, since it blocks the possibility to cache the parse-tree. \define ... Should be replaced by \procedure

We will need to replace $variable-name$ in all our existing examples with the new type of text substitution. The same is true for the core code.

CodaCodr commented 1 year ago

$btn-text$ ... This type of text-substitution should not be used anymore

What a pain. I have many hundreds of those (at a guess).

The release note for 5.3.0 (which, I assume, is still relevant for 5.3.1) says:

The approach taken by this release is to add new functionality by extending and augmenting the system without disturbing existing functionality.

I think it more than fair to say, \define and $my-param$ are, in the context, "existing functionality". Seriously, this is a breaking (bricking?) change. My concern is, there is something lying in wait that, when revealed, will render the UI unusable. I'm struggling to see a way forward...

@Jermolene ?

rmunn commented 1 year ago

I checked with a copy of TW 5.2.7, and the behavior has not changed. What has changed is the content of the $:/core/images/down-arrow tiddler: in TW 5.3.0, a pragma was added that wasn't there in 5.2.7, and your my-btn macro is showing the pragma. Thing is, if you load a wiki file that was made with 5.2.7 or earlier and add a pragma to the $:/core/images/down-arrow tiddler, it will show up in your my-btn macro as well: I added \whitespace trim to the start of $:/core/images/down-arrow and it showed up in the button text.

The reason why is because of the way you're calling the macro and passing it the transclusion. As you discovered, adding quotes around the transclusion works, because adding quotes is the correct way to do it, and calling it without the quotes was actually the wrong way to do it — but you didn't find out that it was wrong until TW 5.3.0 edited the content of the tiddler you're using. I'll explain.

When you do attr={{SomeTiddler}}, TW takes the content of SomeTiddler and passes its content, verbatim with no further wikitext processing, as the value of the attribute. https://tiddlywiki.com/#Transcluded%20Attribute%20Values says "The value of the attribute value will be the exact text retrieved from the TextReference. Any wiki syntax in that text will be left as-is." Because the tiddler you were using had no TW syntax in it, you didn't notice the problem — but if you had used a tiddler for your button that contained Some ''bold'' text, you would be expecting to see "Some bold text" and instead you would have seen "Some ''bold'' text", preciesely because the attr={{SomeTiddler}} does no further wikitext processing. (That's because often, no further processing is exactly what you want, for example when you need to pass some filter text into a macro that will apply the filter.)

The "workaround" you discovered is actually a fix for the bug in your wiki. By doing attr="{{SomeTiddler}}", you ensure that the literal text {{SomeTiddler}} is passed to the attribute. Then that text is substituted into your macro, and wikitext processing happens later. By that point, the wikitext being processed looks like <$button>{{$:/core/images/down-arrow}}</$button>, which means that the $:/core/images/down-arrow tiddler will be transcluded, wikitext-processed, and then used as the button text.

Does that make sense? This isn't a behavior change in 5.3.0; it always worked that way, you just happened to have chosen a transclusion target that didn't have any wikitext in it, so the bug in your macrocall invocation wasn't immediately obvious.

As for replacing those $param$ you have all over the place, that's still a good idea, for efficiency's sake: procedure will work faster than \define, so you'll see a speedup once you make the switch. (Depending on the size of your wiki, it's possible the speedup will only be a few milliseconds, or it might be large enough to notice). But the \define and $param$ features will still work (as long as you've gotten your usage right). So you can take your time about modernizing your wiki code, and switch just one or two macros at a time, starting with the ones you use most often because that's where you'll see the biggest speed gains.

CodaCodr commented 1 year ago

@rmunn

Does that make sense?

Painstakingly, absolutely, crystal clear. Bravo and many thanks.

As for replacing those $param$ you have all over the place, that's still a good idea, for efficiency's sake...

I get that. My "tears" were how many I have, in dozens of wikis. "Luckily" ~900 of them are in one, big, fat (@pmario) bundle. Hopefully, I can put the time in (like a day++?) and get ~90% of them in one go.

But the \define and $param$ features will still work

Thank <insert deity of choice> for that! These wikis are pretty big, on the order of ~5k-10k tiddlers. They share that bundle I mentioned, where most of the macros (one day procs) reside.

I was pretty forlorn after reading pmario's reply -- figured I'd missed something vital during the months of preamble leading up to 5.3.x.

Thanks. Seriously.

pmario commented 1 year ago

I was pretty forlorn after reading pmario's reply -- figured I'd missed something vital during the months of preamble leading up to 5.3.x.

I did miss it too. I had the same problem in my Script-Manager edition, which I do use on a daily basis.

------- slightly OP

There is a "funny" coincidence. The script-manager also is a bundle, that I used to create a video-series in 2017. Now I took the project to create a new video-series with 4 videos. 1 shows how to fix that problem. 1 shows: adding some new functionality and 2 show, how to prepare TW to be a dev system.

Jermolene commented 1 year ago

Hi @CodaCodr with respect to the OP, the only thing that has changed is the format of the core image tiddlers which causes issues in this usage. You can adapt the technique used in the splash screen to workaround the problem.

Notwithstanding @pmario's critique of the code in the OP, the existing macro call syntax is not going to go away any time soon. We recognise the value of existing code, and don't want to compromise that value.