extend lookup op flexibility with 2 parameters

[joshuafontany]

Resolves #5307 by extending the lookup operator to accept 1 or 2 parameters.

• 1 parameter lookup always defaults to retrieving the value from the text field.
• 2 parameters parses the 2nd parameter as a field-name by default, unless it starts with !! or ##. It then looks up the Text Reference created.

Tested and fully compatible with even my JsonMangler extended Json syntax for indexes. Docs updated - for 5.1.23 because hey, is this worth going into this release? :)

[saqimtiaz]

@joshuafontany thank you. A bit too late in the day for 5.1.23 though in my opinion. We really have to resist the urge to slip in last minute changes without enough of a testing period.

[saqimtiaz]

We should add some tests for the new behaviour as well.

[AnthonyMuscio]

@joshuafontany
Thanks so much for implementing this. Since it missed 5.1.23 will it appear in the pre-release?

Or can I somehow apply these to my own wiki for design and testing?

Regards
Tones

[Jermolene]

Thanks so much for implementing this. Since it missed 5.1.23 will it appear in the pre-release?

Or can I somehow apply these to my own wiki for design and testing?

The change is now in the current prerelease and will appear in the v5.1.24 when it is released.

It is not recommended to cherry pick updated JS files from one version of TiddlyWiki to try to use them in an earlier version.

[saqimtiaz]

@Jermolene just to clarify, this has not been merged yet.

@joshuafontany seems like we have a small conflict due to a recently merged fix to the lookup operator for when no default is specified.

[Jermolene]

@Jermolene just to clarify, this has not been merged yet.

Hahaha! Not a great start to the day, I'll go and have some more coffee.

[joshuafontany]

@saqimtiaz @Jermolene upstream merged in. :) I put the same catch in each .push() call.

[Jermolene]

Thanks @joshuafontany

Check if the 2nd parameter starts with !! or ##, else interpret it as a field-name and prefix it with !!

I'm not wildly keen on this sort of polymorphism. It can be brittle, particularly if user supplied text is used for the 2nd operand. I'd favour an explicit additional textual suffix to switch between field and index mode: lookup:default:field[prefix][fieldname] and lookup:default:index[prefix][index].

(Just as background, I now consider it was a mistake to implement any of the !! and ## syntax at the widget level; it should just be a parser thing).

[joshuafontany]

@Jermolene Aaah, thank you Jeremy. That helps me plan future widgets, etc.

I have modified the code to check for an explicit ":index" second-suffix. If missing or any other word, it defaults to "field" mode. If only 1 parameter exists, the default target-field is "text" and default target-index is "0" (zero). I also tried to modify the filter-tests, but messed that up some-how. Need to read the whole test edition at some point...

I hope this can get merged in soon. I am also fixing the filesystem bugs I found, but that has gotten "interesting". :)

[pmario]

Hmmm, I'm not sure, if the "old" behaviour should be overwritten. I think there needs to be "from 5.1.15" AND a "from 5.1.24"

Since TW 5.1.23 uses the description 5.1.15 ..

[pmario]

You may move "from 5.1.24" to the top and "from 5.1.15" to the bottom of the tiddler

[Jermolene]

Hi @joshuafontany I think @pmario is right, it would be helpful to retain the old version marker, if we can distinguish between the features to which they apply.

[saqimtiaz]

bump

[joshuafontany]

bump

@saqimtiaz @Jermolene Thanks for the bump. Any polish needed on this? @pmario's docs suggestion?

[Jermolene]

Thanks @joshuafontany just one minor tweak.

[Jermolene]

Thanks @joshuafontany just one minor tweak.

[saqimtiaz]

@Jermolene the requested documentation changes are in place. This is good to merge.

[Jermolene]

Great stuff, thanks @joshuafontany @saqimtiaz

[Jermolene]

Hi @joshuafontany @saqimtiaz I've now found that this change breaks some of my existing wikis.

Try this on https://tiddlywiki.com/

<$list filter="[all[tiddlers+shadows]tag[$:/tags/PageControls]]">
<div>
<$text text=<<currentTiddler>>/>: <$text text={{{ [<currentTiddler>lookup:show[$:/config/PageLayout/MainColumn/Visibility/]match[show]]"  }}}/>
</div>
</$list>

The results are:

$:/core/ui/Buttons/home: show
$:/core/ui/Buttons/close-all: show
$:/core/ui/Buttons/fold-all: show
$:/core/ui/Buttons/unfold-all: show
$:/core/ui/Buttons/permaview: show
$:/core/ui/Buttons/new-tiddler: show
$:/core/ui/Buttons/new-journal: show
$:/core/ui/Buttons/new-image: show
$:/core/ui/Buttons/import: show
..etc

Then try it on https://tiddlywiki.com/prerelease – the results are:

$:/core/ui/Buttons/home: "
$:/core/ui/Buttons/close-all: "
$:/core/ui/Buttons/fold-all: "
$:/core/ui/Buttons/unfold-all: "
$:/core/ui/Buttons/permaview: "
$:/core/ui/Buttons/new-tiddler: "
$:/core/ui/Buttons/new-journal: "
$:/core/ui/Buttons/new-image: "
$:/core/ui/Buttons/import: "

So, I'm reverting this change for the moment. It would be helpful to have some tests before we try again.

[saqimtiaz]

@Jermolene yikes. Apologies, I missed that there are no tests included. I'll try to find time to review this soon if @joshuafontany doesn't get around to it first.

[saqimtiaz]

I believe the issue is here. If the targetTiddler does not exist, we need to return the default value if one has been specified.

I recommend that we add tests though @joshuafontany

[joshuafontany]

Good catch guys. I have reviewed the code and the code-paths called by the index vs field cases, and yes, it was an issue of a missing tiddler in the field case.

In this case, it seems my recent changes are not copied to the PR. Would I reopen a new PR? What type of tests would we like? Those are built in the Test Edition, right? I feel that recreating the expected results of all of the "lookup Operator Examples" filters to be a good base tests, as the operator is actually used more than elsewhere.