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?] - missing closing tag for vars widget in core macros #7697

Closed twMat closed 1 year ago

twMat commented 1 year ago

Describe the bug

Bug or nothing?

https://tiddlywiki.com/#%24%3A%2Fcore%2Fmacros%2Ftag

...the tag-pill-inner macro starts off with a <$vars> widget but there is no closing </$vars> tag.

(Please do close this if it is nothing.)

ericshulman commented 1 year ago

By default, any widgets that are "unclosed" when the end of a macro (or end of a tiddler) is encountered are implicitly "closed". While this might seem like a bug, omitting closing tags can actually be very handy.

For example, suppose we have a macro definition that starts with a $list widget followed by a few $set, $vars and $let widgets that create some internal "working variables" for use within the macro. Rather than having to match each with a corresponding /$let, /$vars, /$set and /$list (in reverse order, since the widgets are effectively nested!), we can just end the macro.

twMat commented 1 year ago

Ah, thanks @ericshulman - I knew this about end of tiddlers but not also for macros... which does seem odd considering how macros are just text substitution and they can be called from anywhere in the tiddler. It also seems inconsistent that the very macro in that very tiddler which I refer to, very explicitly includes the end tag for an inner nested widget (the </$element-tag$>).

Anyway, thanks. Closing.

pmario commented 1 year ago

I think this is an oversight. While Eric is right, we usually do close all widgets in the core as an example for best practice. Users often do copy paste our code into their own creations and missing end-markers can lead to problems.

@Jermolene -- What do you think. Should we fix this?

twMat commented 1 year ago

Should we fix this?

If we do, may I propose that while we're at it, exchange $vars for $let - and that we do this generally from now on when modifying core tids? I assume that specifically the $let widget cannot be easily "search-replaced". Again, for setting an example of what is best practice. Thoughts on this?

pmario commented 1 year ago

Thoughts on this?

I'm not sure if there is a "grand strategy" for modernizing TW atm.

I know, there is a commitment from @Jermolene that we want to update major aspects of the TW core code. Wikitext and Javascript -- but there is no timeline

For new wikitext tiddlers Jeremy lately demanded the new syntax -- but for existing code its much more complex, because many UI elements are kind of "interconnected". IMO they should be done at once.

So even if you start small, those PRs tend to "run out of scale" very fast. Eg: PR #7657 -- The first PR created in March this year changed 81 docs only tiddlers and it was not merged yet. :(

Jermolene commented 1 year ago

Thank you @twMat that is indeed a bug. It is now fixed in fa9bfa07a095548eb2f8339b0b1b816d2e6794ef; I took up your suggestion of turning the vars widget into a let widget