$parameters widgets now claim index parameters sequentially (#7962)

* $parameters widgets now claim index parameters sequentially

Before, they just always used their own indices, which meant unnamed
parameters would get assigned to multiple values, which wasn't any
use to anyone.

* Elaborated tests for nested parameters

* Added example to clarify how multiple paramter definitions are handled

* Updated \procedure documentation

* Removed depth from $parameters

* Add failing test

* Adjusted transclude to account for refreshing

* This index system should work

It handles some strange edge cases too.

* Stupid end-of-file newlines

* Removed redundant whitespace pragma

---------

Co-authored-by: Jeremy Ruston <jeremy@jermolene.com>

core/modules/widgets/parameters.js

@@ -42,31 +42,24 @@ Compute the internal state of the widget
 */
 ParametersWidget.prototype.execute = function() {
 	var self = this;
-	this.parametersDepth = Math.max(parseInt(this.getAttribute("$depth","1"),10) || 1,1);
 	// Find the parent transclusions
-	var pointer = this.parentWidget,
+	var pointer = this.getContainingTransclude();
-		depth = this.parametersDepth;
-	while(pointer) {
-		if(pointer instanceof TranscludeWidget) {
-			depth--;
-			if(depth <= 0) {
-				break;
-			}
-		}
-		pointer = pointer.parentWidget;
-	}
 	// Process each parameter
-	if(pointer instanceof TranscludeWidget) {
+	if(pointer) {
+		// It's important to remember this, because when we refresh, we'll need to make sure this widget is starting at the same index.
+		this.initialParameterIndex = pointer.parameterIndex;
 		// Get the value for each defined parameter
-		$tw.utils.each($tw.utils.getOrderedAttributesFromParseTreeNode(self.parseTreeNode),function(attr,index) {
+		$tw.utils.each($tw.utils.getOrderedAttributesFromParseTreeNode(self.parseTreeNode),function(attr) {
 			var name = attr.name;
 			// If the attribute name starts with $$ then reduce to a single dollar
 			if(name.substr(0,2) === "$$") {
 				name = name.substr(1);
 			}
-			var value = pointer.getTransclusionParameter(name,index,self.getAttribute(attr.name,""));
+			var value = pointer.getTransclusionParameter(name,self.getAttribute(attr.name,""));
 			self.setVariable(name,value);
 		});
+		// We remember where we left the unnamed parameter index.
+		this.finalParameterIndex = pointer.parameterIndex;
 		// Assign any metaparameters
 		$tw.utils.each(pointer.getTransclusionMetaParameters(),function(getValue,name) {
 			var variableName = self.getAttribute("$" + name);
@@ -84,13 +77,29 @@ Refresh the widget by ensuring our attributes are up to date
 */
 ParametersWidget.prototype.refresh = function(changedTiddlers) {
 	var changedAttributes = this.computeAttributes();
-	if(Object.keys(changedAttributes).length) {
+	var pointer = this.getContainingTransclude();
+	var currentParameterIndex;
+	if(pointer) {
+		currentParameterIndex = pointer.parameterIndex;
+	}
+	if(Object.keys(changedAttributes).length || currentParameterIndex !== this.initialParameterIndex) {
 		this.refreshSelf();
 		return true;
+	} else if(pointer) {
+		// We set the index for unnamed parameters for our $transclude widget in case any later $parameters show up. They need to be able to confirm their indices are starting in the right place, because if not, they need to refresh.
+		pointer.parameterIndex = this.finalParameterIndex;
 	}
 	return this.refreshChildren(changedTiddlers);
 };
 
+ParametersWidget.prototype.getContainingTransclude = function() {
+	var pointer = this.parentWidget;
+	while(pointer && !(pointer instanceof TranscludeWidget)) {
+		pointer = pointer.parentWidget;
+	}
+	return pointer;
+};
+
 exports.parameters = ParametersWidget;
 
 })();

core/modules/widgets/transclude.js

@@ -357,15 +357,27 @@ TranscludeWidget.prototype.getOrderedTransclusionParameters = function() {
 	return result;
 };
 
+/*
+The parameter index indicates is used by internal $parameter widgets to sequentially assign unnamed parameters.
+*/
+Object.defineProperty(TranscludeWidget.prototype, 'parameterIndex', {
+    get: function() { return this.claimedIndices || 0; },
+    set: function(value) { this.claimedIndices = value; }
+});
+
 /*
 Fetch the value of a parameter given either its name or index
 */
-TranscludeWidget.prototype.getTransclusionParameter = function(name,index,defaultValue) {
+TranscludeWidget.prototype.getTransclusionParameter = function(name,defaultValue) {
 	if(name in this.stringParametersByName) {
 		return this.stringParametersByName[name];
 	} else {
+		// Let's see if this name was already assigned an index
+		var index = this.parameterIndex;
 		var name = "" + index;
 		if(name in this.stringParametersByName) {
+			// This parameter now corresponds to this index. No other parameters may correspond to it.
+			this.claimedIndices = index + 1;
 			return this.stringParametersByName[name];
 		}
 	}
@@ -448,6 +460,8 @@ Selectively refreshes the widget if needed. Returns true if the widget or any of
 */
 TranscludeWidget.prototype.refresh = function(changedTiddlers) {
 	var changedAttributes = this.computeAttributes();
+	// Reset the parameter index so that internal $parameter widgets can double-check their unnamed parameter indices.
+	this.parameterIndex = 0;
 	if(($tw.utils.count(changedAttributes) > 0) || (this.transcludeVariableIsFunction && this.functionNeedsRefresh()) || (!this.transcludeVariable && changedTiddlers[this.transcludeTitle] && this.parserNeedsRefresh())) {
 		this.refreshSelf();
 		return true;

editions/test/tiddlers/tests/data/transclude/Parameterised-Conditional-Parameters-Refreshed.tid

@@ -0,0 +1,38 @@
+title: Transclude/Parameterised/ConditionalParameters/Refreshed
+description: Parameterised transclusion when conditional parameters are changed by a refresh
+type: text/vnd.tiddlywiki-multiple
+tags: [[$:/tags/wiki-test-spec]]
+
+title: Output
+
+\whitespace trim
+\procedure elephant(filler)
+<$let name={{Canary}}>
+<% if [<name>match[yes]] %>
+<$parameters A=defaultA>
+A/<$text text=<<filler>>/>/<$text text=<<A>>/>/<$text text=<<B>>/>
+</$parameters>
+<% else %>
+<$parameters B=defaultB>
+B/<$text text=<<filler>>/>/<$text text=<<A>>/>/<$text text=<<B>>/>
+</$parameters>
+<% endif %>
+\end elephant
+
+-
+<<elephant ignore A:myA B:myB>>
+&#32;&#32;
+<<elephant ignore myA myB>>
+
++
+title: Canary
+
+no
++
+title: Actions
+
+<$action-setfield $tiddler="Canary" text="yes"/>
++
+title: ExpectedResult
+
+<p>-A/ignore/myA/  A/ignore/myA/</p>

editions/test/tiddlers/tests/data/transclude/Parameterised-Conditional-Parameters.tid

@@ -0,0 +1,30 @@
+title: Transclude/Parameterised/ConditionalParameters
+description: Parameterised transclusion with conditional parameters
+type: text/vnd.tiddlywiki-multiple
+tags: [[$:/tags/wiki-test-spec]]
+
+title: Output
+
+\procedure elephant(filler)
+\whitespace trim
+<% if [[no]match[yes]] %>
+<$parameters A=defaultA>
+A/<$text text=<<filler>>/>/<$text text=<<A>>/>/<$text text=<<B>>/>
+</$parameters>
+<% else %>
+<$parameters B=defaultB>
+B/<$text text=<<filler>>/>/<$text text=<<A>>/>/<$text text=<<B>>/>
+</$parameters>
+<% endif %>
+\end elephant
+\whitespace trim
+
+-
+<<elephant ignore A:myA B:myB>>
+&#32;&#32;
+<<elephant ignore myB not-used>>
+
++
+title: ExpectedResult
+
+<p>-B/ignore//myB  B/ignore//myB</p>

editions/test/tiddlers/tests/data/transclude/Parameterised-Depth.tid

@@ -1,34 +0,0 @@
-title: Transclude/Parameterised/Depth
-description: Parameterised transclusion using the $depth attribute
-type: text/vnd.tiddlywiki-multiple
-tags: [[$:/tags/wiki-test-spec]]
-
-title: Output
-
-\whitespace trim
-<$transclude $tiddler='TiddlerOne' one='Ferret'/>
-|
-<$transclude $tiddler='TiddlerOne'/>
-|
-<$transclude $tiddler='TiddlerOne' one='Ferret' $$two="Osprey"/>
-|
-<$transclude $tiddler='TiddlerOne' $$two="Falcon"/>
-+
-title: TiddlerOne
-
-\whitespace trim
-{{TiddlerTwo}}
-+
-title: TiddlerTwo
-
-\whitespace trim
-<$parameters one='Jaguar' $$two='Piranha' $depth="2">
-	<$text text=<<one>>/>:<$text text=<<$two>>/>
-</$parameters>
-<$parameters one='Leopard' $$two='Coelacanth'>
-	(<$text text=<<one>>/>|<$text text=<<$two>>/>)
-</$parameters>
-+
-title: ExpectedResult
-
-<p>Ferret:Piranha(Leopard|Coelacanth)|Jaguar:Piranha(Leopard|Coelacanth)|Ferret:Osprey(Leopard|Coelacanth)|Jaguar:Falcon(Leopard|Coelacanth)</p>

editions/test/tiddlers/tests/data/transclude/Parameterised-Genesis-Changing-Count.tid

@@ -0,0 +1,38 @@
+title: Transclude/Parameterised/GenesisChangingCount
+description: Parameterised transclusion using genesis can handle refreshes that change number of parameters
+type: text/vnd.tiddlywiki-multiple
+tags: [[$:/tags/wiki-test-spec]]
+
+title: Output
+
+\procedure elephant(filler)
+\whitespace trim
+<$text text=<<filler>>/>-
+<$let args={{Canary}}>
+<$genesis $type=$parameters $names="[enlist<args>]" $values="[enlist<args>addsuffix[-default]]">
+<$text text=`($(argA)$-$(argB)$-$(argC)$)` />
+</$genesis>
+</$let>
+<$parameters other=other-default>
+-<$text text=<<other>>/>
+\end elephant
+
+-
+<<elephant filler argA:myA argB:myB argC:myC other:myOther>>
+<<elephant filler myA myB myC myOther>>
+
++
+title: Canary
+
+argA argB
++
+title: Actions
+
+<$action-setfield $tiddler="Canary" text="argA argB argC"/>
++
+title: ExpectedResult
+
+<p>-
+filler-(myA-myB-myC)-myOther
+filler-(myA-myB-myC)-myOther
+</p>

editions/test/tiddlers/tests/data/transclude/Parameterised-Genesis-Changing-Names.tid

@@ -0,0 +1,34 @@
+title: Transclude/Parameterised/GenesisChangingNames
+description: Parameterised transclusion using genesis can handle refreshes that change parameter names
+type: text/vnd.tiddlywiki-multiple
+tags: [[$:/tags/wiki-test-spec]]
+
+title: Output
+
+\procedure elephant(filler)
+\whitespace trim
+<$let name={{Canary}}>
+<$genesis $type=$parameters $names="[<name>]" $values="default">
+<$text text=<<First>>/>/<$text text=<<Second>>/>
+</$genesis>
+</$let>
+\end elephant
+\whitespace trim
+
+-
+<<elephant ignore First:init Second:var>>
+-
+<<elephant ignore var>>
+
++
+title: Canary
+
+First
++
+title: Actions
+
+<$action-setfield $tiddler="Canary" text="Second"/>
++
+title: ExpectedResult
+
+<p>-/var-/var</p>

editions/test/tiddlers/tests/data/transclude/Parameterised-Nested-Parameters-Refreshed.tid

@@ -0,0 +1,44 @@
+title: Transclude/Parameterised/NestedParameters/Refreshed
+description: Parameterised transclusion with nested parameter widgets and a refresh cycle
+type: text/vnd.tiddlywiki-multiple
+tags: [[$:/tags/wiki-test-spec]]
+
+title: Output
+
+\procedure elephant(first:"one",second:"two")
+\whitespace trim
+\parameters (third:"three")
+<$let variable={{Canary}}>
+<$parameters fourth=four>
+<$text text=<<first>>/>/<$text text=<<second>>/>/<$text text=<<third>>/>/<$text text=<<fourth>>/>
+</$parameters>
+</$let>
+\end elephant
+
+Begin
+<<elephant>>
+<<elephant "a" "b" "c" "d">>
+<<elephant second:named a c d>>
+<<elephant third:3rd second:2nd a d>>
+<<elephant fourth:4th>>
+<<elephant a fourth:4th>>
+
++
+title: Canary
+
+Else
++
+title: Actions
+
+<$action-setfield $tiddler="Canary" text="Something"/>
++
+title: ExpectedResult
+
+<p>Begin
+one/two/three/four
+a/b/c/d
+a/named/c/d
+a/2nd/3rd/d
+one/two/three/4th
+a/two/three/4th
+</p>

editions/test/tiddlers/tests/data/transclude/Parameterised-Nested-Parameters.tid

@@ -5,18 +5,30 @@ tags: [[$:/tags/wiki-test-spec]]
 
 title: Output
 
-\whitespace trim
 \procedure elephant(first:"one",second:"two")
+\whitespace trim
 \parameters (third:"three")
-\parameters (fourth:"four")
+<$parameters fourth=four>
 <$text text=<<first>>/>/<$text text=<<second>>/>/<$text text=<<third>>/>/<$text text=<<fourth>>/>
+</$parameters>
 \end elephant
 
+Begin
 <<elephant>>
--
 <<elephant "a" "b" "c" "d">>
+<<elephant second:named a c d>>
+<<elephant third:3rd second:2nd a d>>
+<<elephant fourth:4th>>
+<<elephant a fourth:4th>>
 
 +
 title: ExpectedResult
 
-<p>one/two/three/four</p><p>-a/b/c/d</p>
+<p>Begin
+one/two/three/four
+a/b/c/d
+a/named/c/d
+a/2nd/3rd/d
+one/two/three/4th
+a/two/three/4th
+</p>

editions/tw5.com/tiddlers/pragmas/Pragma_ _procedure.tid

@@ -67,20 +67,8 @@ This is <<name>> demonstrates <<desc>>.
 <<myproc>>
 """>>
 
-!! Caution in Using Positional Parameters
+!! When Using Positional Parameters
-Procedures are a shortcut syntax for the SetVariableWidget with an implicit ParametersWidget, so generally there is no reason to have multiple parameters widgets within a definition. In the below example when passing `x` to `myproc`, it will also be set to `a`:
+It is uncommon to require multiple parameters pragma in a definition, but if you do so, the positions for parameters are assigned sequentially in the order of discovery. In the below example, `a` is the second positional parameter, because it comes after `x`:
-
-<<wikitext-example-without-html 
-src:"""\procedure myproc(x:10)
-\parameters (a:100, b:200)
-
-x=<<x>>, a=<<a>>, b=<<b>>
-\end
-
-<<myproc 50>>
-""">>
-
-The reason for that result is clearer if we consider an equivalent with explicit parameters widgets.
 
 <$macrocall $name=wikitext-example-without-html 
 src='<$let myprog="""
@@ -89,16 +77,26 @@ src='<$let myprog="""
 
 x=<<x>>, a=<<a>>, b=<<b>>
 """>
-<<myprog 50>>
+<<myprog 50 73>>
 </$let>'
 />
 
-This is because those two parameters widgets are entirely independent. They are both processed as if the other parameter widget is not there.
+This behavior is the same when parameters are defined in the procedure's declaration.
+
+<<wikitext-example-without-html
+src:"""\procedure myproc(x:10)
+\parameters (a:100, b:200)
+
+x=<<x>>, a=<<a>>, b=<<b>>
+\end
+
+<<myproc 50 73>>
+""">>
+
+This is because a procedures are a shortcut syntax for the <<.wlink SetWidget>> widget with an implicit <<.wlink ParametersWidget>> widget.
 
 <<.tip "The positional parameters are only required when using the parameterised transclusion shortcut syntax, and that in other cases it is generally clearer to use named parameters.">>
 
-To prevent such situation of above example, pass parameters by name as below.
-
 <<wikitext-example-without-html 
 src:"""\procedure myproc(x:10)
 \parameters (a:100, b:200)
@@ -106,5 +104,5 @@ src:"""\procedure myproc(x:10)
 x=<<x>>, a=<<a>>, b=<<b>>
 \end
 
-<<myproc x:50>>
+<<myproc a:73>>
-""">>
+""">>

editions/tw5.com/tiddlers/widgets/ParametersWidget.tid

@@ -13,17 +13,13 @@ There are shortcuts for common scenarios that can often make it unnecessary to u
 * the [[Pragma: \procedure]] for declaring procedure
 * the [[Pragma: \widget]] for declaring custom widgets
 
-The <<.wlink ParametersWidget>> widget must be used directly in the following situations:
+The <<.wlink ParametersWidget>> widget must be used directly when the default value of a parameter must be computed dynamically.
-
-* When the default value of a parameter must be computed dynamically
-* When the `$depth` attribute is used to retrieve parameters from a parent transclusion (see below)
 
 ! Content and Attributes
 
 The content of the <<.wlink ParametersWidget>> widget is the scope within which the values of the parameters can be accessed as ordinary variables.
 
 |!Attribute |!Description |
-|$depth |The index of the parent transclusion from which to obtain the parameters (defaults to 1). See below |
 |$parseMode |Optional name of a variable in which is made available the parse mode of the content of the parent transclusion (the parse mode can be "inline" or "block") |
 |$parseTreeNodes |Optional name of a variable in which is made available the JSON representation of the parse tree nodes contained within the parent transclusion |
 |$slotFillParseTreeNodes |Optional name of a variable in which is made available the JSON representation of the parse tree nodes corresponding to each fill widget contained within the parent transclusion (as an object where the keys are the slot names and the values are the parse tree nodes) |
@@ -34,9 +30,6 @@ The content of the <<.wlink ParametersWidget>> widget is the scope within which
 
 <<.note "Note the special treatment required for parameters names that start with a `$`; this can be avoided by using one of the pragmas">>
 
-!! `$depth` Attribute
-
-By default, the <<.wlink ParametersWidget>> widget retrieves parameters from the immediate parent transclusion. The `$depth` attribute permits access to the parameters of parent transclusions by specifying an index to the parent to be inspected ("1" is the immediate parent, "2" is the parent of that parent, etc.). This is useful in some situations where an intervening transclusion prevents immediate access to desired parameters.
 
 !! `$parseMode`, `$parseTreeNodes`, `$slotFillParseTreeNodes` and `$params` Attributes
 

editions/tw5.com/tiddlers/widgets/examples/ParametersWidget (Examples).tid

@@ -24,8 +24,7 @@ In this simple form, parameters passed by position not by name. So the first val
 ''Remarks''
 
 # Passing parameter by name is good practice and is recommended for clarity. So for parameterized transclusions, the use of <<.wid transclude>> is recommended over simple form transclusion.
-# When passing parameters value by position, you cannot pass the second parameter while the first one has not been passed.
+# However, if parameter values are passed by position, parameters get assigned sequentially based on the order <<.wlink ParametersWidget>> widgets are discovered.
-
 
 ''Example iv'': Here the <<.wlink ParametersWidget>> widget is used to declare a parameter whose default value is transcluded from another tiddler.
 
@@ -36,4 +35,17 @@ My name is <<name>> and my age is <<age>>.
 \end
 
 <$transclude $variable="myproc" age="19"/>
-"""/>
+"""/>
+
+''Example v'': Here the <<.wlink ParametersWidget>> widget is used in addition to other parameter declarations, because later parameters depend on the value of earlier parameters. Positional values are assigned sequentially.
+
+<$macrocall $name=".example" n="5" eg="""\procedure myproc(age: 22)
+<$parameters acceptable-age={{{ [<age>divide[2]add[7]] }}} >
+If you're <<age>>, you can date a <<acceptable-age>>-year-old and it's not weird.
+</$parameters>
+\end
+
+<<myproc>>
+<<myproc 30>>
+<<myproc 35 70>>
+""" />