[BUG] MVVs are not preserved when used as parameters [5.4.0-prerelease]

[yaisog]

Describe the bug

When assigning an MVV with $let and then using that variable as a procedure parameter, its multi-valued nature is not preserved:

\procedure show-items(itemList)
	<h2>itemList</h2>

	<$list filter="[(itemList)]" counter="id" join="<br>" >
		Item number <<id>> is <<currentTiddler>>
	</$list>

	<h2>items</h2>

	<$list filter="[(items)]" counter="id" join="<br>" >
		Item number <<id>> is <<currentTiddler>>
	</$list>
\end

<$let items={{{ [tag[Learning]] }}} >
	<$transclude $variable="show-items" itemList=<<items>> />
</$let>

This should yield two lists with a large number of items each. However, the first list contains only one item.

A closely related problem is the situation when the items list is empty, since the procedure cannot distinguish between an MVV without any elements and an empty variable.
Within the above procedure, [(items)] will then also be empty, but [(itemList)] or [<itemList>] will be an empty string, so the line

<$list filter="[(itemList)]" counter="id" join="<br>" >

would need to be changed to

<$list filter="[(itemList)!is[blank]" counter="id" join="<br>" >

to mostly function the same. However, that will interfere with situations where one of the items can actually be blank (for testing use <$let items={{{ [[]] [tag[Learning]] }}} >).

Expected behavior

MVVs should keep all their properties when used in procedures (and functions).

To Reproduce

Use the above examples on https://tiddlywiki.com/prerelease/

Screenshots

No response

TiddlyWiki Configuration

• Version 5.4.0-prerelease

Additional context

No response

[pmario]

IMO That's expected: <$transclude $variable="show-items" itemList=<<items>> /> The <<items>> variable definition in the transclude call only contains the first element. That's per definition for backwards compatibility.

$let is the only widget that allows us to define MVVs. Macro calls are not

[Jermolene]

Thanks @yaisog @pmario the following test that is part of #8972 shows how MVVs can be passed to a function:

\function myfunc() [all[tiddlers]sort[]]

<$let varname=<<myfunc>>>
<$text text={{{ [(varname)] +[join[-]] }}}/>
</$let>

Procedures do not currently support MVV parameters so I think @yaisog's suggestion makes sense in order to harmonise things. @yaisog would you be interested in picking that up?

[yaisog]

the following test that is part of #8972 shows how MVVs can be passed to a function

Hi @Jermolene, doesn't that show how to return an MVV from a function, not pass to it?
I meant something more like this

\function test(itemList) [(itemList)join[-]]

<$let items={{{ [tag[Learning]] }}} >
	<$transclude $variable="test" itemList=<<items>> />
</$let>

It does work with the function[] operator in filters:

<$let items={{{ [tag[Learning]] }}} >
	{{{ [function[test],(items)] }}}
</$let>

That's another inconsistency that we should probably tackle.

Would you be interested in picking that up?

I'll take a look. I am more versed with filter operators and the like, though.

[Jermolene]

Hi @yaisog apologies I intended to paste this test excerpt:

\function my.func(input) [(input)sort[]]

<$let
	input={{{ [all[tiddlers]] }}}
	output={{{ [my.func(input)] }}}
>
<$text text={{{ [(output)] +[join[-]] }}}/>
</$let>

The point I was drawing is that in order to pass an MVV to a function within a filter it is necessary to use the round brackets to signify that the parameter is an MVV. If angle brackets were used instead then the behaviour is constrained by backwards compatibility, and could only mean that a single value is being passed.

Another way of saying the same thing is that your first example could never work for passing an MVV because it already has a well defined meaning pre-v5.4.0.

<$let items={{{ [tag[Learning]] }}} >
	<$transclude $variable="show-items" itemList=<<items>> />
</$let>

[Jermolene]

I'll take a look. I am more versed with filter operators and the like, though.

I'd welcome another set of eyes on the code. It's one of the more complex enhancements we've done because it has so many touchpoint with the core. There's a high chance of things falling through the cracks.

[yaisog]

Hi @Jermolene, looking at the involved widgets, to make this work with procedure parameters, I would probably start with the TranscludeWidget and run its computeAttributes() call with the asList option, like the LetWidgt now does, or something silimar. Then check if I get back an MVV or a regular variable and pass this to the generated ParametersWidget (which would need corresponding extensions?). Then see where that leads...
Does this sound about right or do I need to think it over?

your first example could never work for passing an MVV because it already has a well defined meaning pre-v5.4.0

I don't think I know what you mean exactly. If (a modified) $transclude would pass the MVV items as itemList into the function, and the function were to handle it properly as the LetWidget does, then both <itemList> (pre-5.4.0 syntax) and (itemList) would work, as if there was a LetWidget inside the function scope. I can't see the syntax collision, but I'm still fighting through a thick jungle of code and don't see that much at all... 😉

[Jermolene]

Hi @yaisog on reflection, I think you're right, and it is currently consistent with the let widget to use name=<<mvv>> to pass an MVV to a procedure.

The inconsistency that I'm finding frustrating also occurs with the let widget. We use <name> within filters to retrieve a single value, and (mvvname) to retrieve a list of results from an MVV. We purposely establish the rule of thumb that round brackets are needed to retrieve all the results in an MVV, and angle brackets only retrieve the first result. But the let widget treats name=<<mvvname>> as an assignment of all the results, breaking the rule of thumb.

So, I think my proposal applies both to the putative extensions to the transclude widget, and the existing handling by the let widget. It is basically just to make =<<name>> always assign a single result, and =((mvvname)) to assign all the values of an MVV. It's a change from where we are at the moment, but it might make things less ambiguous.

[yaisog]

Hi @Jermolene, do you really want to reopen this metaphorical can of worms? I remember a very lengthy discussion on TalkTW about that where everyone seemed to have a different opinion...

I do agree that there is some disconnect when using <<variable>> with an MVV. However, in that context (of parameter assignment) you don't really know or care that it's an MVV, you just want itemList to be the same as items, like it currently works with the LetWidget.
Whether it's an MVV or not is not relevant when passing it, but only when you actually use it. And right now, and foreseeably, this will be in filters, where there is an explicit syntax for it. If you unknowingly or uncaringly made an MVV into a regular variable during the transfer, you'll have a harder time debugging that.

Also, we'd then have to make and implement rules what it should mean to write ((name)) in plain WikiText. Should this output the whole list? With some template? What if it's used as a widget parameter? What would <$text text=((items)) /> do? Or <$edit-text tiddler=((titleList)) />? Probably, you'd only really be able to use it with $let and $transclude. I'd rather document the caveat that for the TranscludeWidget and LetWidget foo=<<bar>> will copy MVVs as-is than documenting for all other widgets that foo=((bar)) is not valid / meaningful syntax.

Establishing a new syntax like the double parentheses and thus requiring explicit variable type choices everywhere is a bit like the old $name$ or even $(name)$ syntax (we did have parentheses before!). I remember having a really hard time getting my macros to work, it was just an endless trying of different combinations of $,( and <. This is so much cleaner now. I wouldn't want to spoil that. I'll do it if need be to be able to pass MVVs to procedures, but begrudgingly. 😉

[yaisog]

Hi @Jermolene, sleeping on it, I think maybe you're right. It is cleaner to intentionally and explicitly assign all values instead of only the first one. There are things to be considered, though:

• ((shortcut)) is already used for keyboard shortcut references. This would show up in WikiText like <$keyboard key="((my-shortcut))" ... (in quotes). I do not think it's a feature that is often used and this conflict may be acceptable, but still we would at least need a warning in the docs.
• We need a plan how to handle the case when users combine the parentheses syntax with widgets that don't support them, i.e. all of them except $transclude and $let. Issue a warning, an error or silently use only the first value?

@AnthonyMuscio proposed the alternative syntax <(name)> on TalkTW which we might also consider, in which case only the innermost delimiter distinguishes between regular variables and MVVs, similar to a filter like [(name)] vs. [<name>]. This would solve the first point above, but not the second.

[Jermolene]

Thanks @yaisog. It is slightly galling to be reopening this discussion but I guess that is another consequence of the depth and breadth of these changes. Perhaps it's better to think of it as a deliberate broadening of the scope of MVVs, and thus a consequence of our ambition...

The keyboard shortcut thing is annoying, particularly as we needn't have used any kind of brackets here, a special prefix would have been sufficient. But as you suggest, it's a conceptually narrow and simple ambiguity as these things go.

Opening the can of worms even wider, we could decorate the equals sign used in attribute assignments to indicate that a list of results should be assigned. We'd still have the problem that these assignments would only work for some attributes of some widgets. It would emphasise that the new behaviour of assigning a full list of results is a property of the assignment itself, not the of the value expression. We would also need to be mindful of the decorated equals signs used by common programming languages might rule out obvious and easily typed choices like ==. The more serious problem is the redundancy: to assign an MVV would still require the use of double parentheses, as well as the special equals sign. So, I think this is a dead end but it might trigger other ideas.

The more I think about it, it seems impossible to devise any syntax extensions for MVV assignments that don't suffer from the same fundamental problem of needing to know a priori which attributes of which widgets take MVV values. It seems fairly obvious that attempting to assign an MVV value to a single valued attribute would just assign the first result in the list. But that would mean that parentheses and angle brackets would be interchangeable in widget assignments, but not within filters. Maybe that's a price worth paying for a better defined and more consistent overall design.

I don't think we need to resolve this before proceeding with the changes we've discussed, which is again very narrow because it only touches widget assignment. As it happens that code is getting hairy and so it could be a motivation to do some refactoring (I think @linonetwo made a proposal here).

[yaisog]

But that would mean that parentheses and angle brackets would be interchangeable in widget assignments, but not within filters.

I was going to answer that this isn't so, but checked first to see what

[tag[Learning]] =>[[titleList]] [tag[HelloThere]prefix<titleList>]

does. I was actually very surprised to see an empty result. I would have expected the prefix operator to use the first item in the titleList MVV, just like the title[] operator does. It doesn't even work if there is only a single item:

[[--]] =>[[titleList]] [tag[HelloThere]prefix<titleList>]

yields no results.
This should be remedied if at all possible to bring this on par with attribute assignments.

Assuming this a bug that will be fixed, prefix(titleList) will still not work, and doesn't need to. But here we at least have the same situation as with the ((name)) syntax and attributes: It won't work in every combination. Just as there are only a few operators that will handle MVVs, there will only be a few widgets that do. And they use a special syntax for that. That's consistent, at least.

[pmario]

@yaisog wrote:

I do agree that there is some disconnect when using <<variable>> with an MVV. However, in that context (of parameter assignment) you don't really know or care that it's an MVV, you just want itemList to be the same as items, like it currently works with the LetWidget.
Whether it's an MVV or not is not relevant when passing it, but only when you actually use it. And right now, and foreseeably, this will be in filters, where there is an explicit syntax for it. If you unknowingly or uncaringly made an MVV into a regular variable during the transfer, you'll have a harder time debugging that.

I think this idea: "MVV or not is not relevant when passing it, but only when you actually use it" -- is interesting. I think it is true.

If there are MVVs we should internally work with them.

1. Then users can decide, how to use them [<variable>] or [(mvv)]
2. Widgets can make "sensible" choices by default
   2.1 eg: a yes / no input only uses the first element. disabled=<<mvv>>
   2.2 A "filter" input uses all of them. eg: filter=<<mvv>>

I think that's an idea to explore. It may keep the syntax simple, allows users to choose. It will need some thinking about the default use 2.1) and 2.2) ...

Just some thoughts

[Jermolene]

This should be remedied if at all possible to bring this on par with attribute assignments.

Assuming this a bug that will be fixed, prefix(titleList) will still not work, and doesn't need to. But here we at least have the same situation as with the ((name)) syntax and attributes: It won't work in every combination. Just as there are only a few operators that will handle MVVs, there will only be a few widgets that do. And they use a special syntax for that. That's consistent, at least.

I agree on both points. Perhaps we're at the point where we could draft an issue bringing together all the proposed changes and bug fixes.