[BUG] calling tag-picker macro with a tagFields=foo shows the tags field instead of foo

[pmario]

Describe the bug

calling tag-picker macro with a tagFields=foo shows the tags field instead of foo

To reproduce

• got to tiddlywiki.com
• copy paste the following code
• if the input is selected it shows the list using the tags field instead of a list of all "foo-fields"

<<tag-picker tagField:foo>> 

Expected behavior

It should use the content of all the foo-fields

[pmario]

Related Talk thread: https://talk.tiddlywiki.org/t/tag-picker-is-not-working-independently/7029

[ericshulman]

The contents of the tag input dropdown list should be configurable, and for backward-compatibility should still default to showing a list of tag values, regardless of the target tagField parameter value. This can be achieved with just a few small changes to the existing $:/core/macros/tag-picker definition:

In the main tag-picker macro definition, change

• \define tag-picker(actions,tagField:"tags")
  to
  \define tag-picker(actions,tagField:"tags",tagFilter:"[tags[]]")
• <$vars saveTiddler=<<currentTiddler>> palette={{$:/palette}}>
  to
  <$vars saveTiddler=<<currentTiddler>> palette={{$:/palette}} tagFilter=<<__tagFilter__>>>

Then, in the tag-picker-inner macro definition, change

• nonSystemTagsFilter="[tags[]!is[system]search:title<userInput>sort[]]"
  to
  nonSystemTagsFilter="[subfilter<tagFilter>!is[system]search:title<userInput>sort[]]"
• systemTagsFilter="[tags[]is[system]search:title<userInput>sort[]]"
  to
  systemTagsFilter="[subfilter<tagFilter>is[system]search:title<userInput>sort[]]"

Also, change field first-search-filter from

• [tags[]!is[system]search:title<userInput>sort[]]
  to
  [subfilter<tagFilter>!is[system]search:title<userInput>sort[]]

and change field second-search-filter from

• [tags[]is[system]search:title<userInput>sort[]]
  to
  [subfilter<tagFilter>is[system]search:title<userInput>sort[]]

With these changes in place, you can then write:

<<tag-picker tagField:foo tagFilter:"[get[foo]enlist-input[]]">>

to populate the dropdown list with all values currently contained within foo fields

[pmario]

@ericshulman ... These changes suggest, that tagFilter becomes a new visible parameter. Right?

That's OK for users, that know what they do. .. What I do not like too much is, that there need to be 2 parameters which depend on each other. ... See the foo field name in the line below.

<<tag-picker tagField:"foo" tagFilter:"[get[foo]enlist-input[]]">>

I would prefer, to be able to call the macro as follows and it should still work.

<<tag-picker tagField:"foo" >>

Did you test it with: \define tag-picker(actions,tagField:"tags",tagFilter:"[get<tagField>enlist-input[]]") as the initial definition?

I think it should work and give us both possibilities. Users can provide a different tagField parameter without the need for a tagFilter. .. But it will still be possible to have a tagFilter if they want to.

What do you think?

[pmario]

There are some more problems: eg: Several instances of the tag-picker in the same tiddler do not work ... And some more

<<tag-picker tagField:"foo">>

<<tag-picker tagField:"bar">>

So the whole thing needs a rewrite.

[pmario]

Having a closer look, I think we should let the "tag-picker" macro alone and create a new "picker-macro" that uses v5.3.0 options.

I think, it would be a good exercise and could be a "real-world" example, to show the differences between v5.2.x code and v5.3.0 code.

We would have no problems with backwards compatibility, since the imo the name "tagField" parameter is completely wrong if used with a different field name. IMO it should be "field". The same is true for the "tagFilter" parameter .. and so on.

There are several $text-substitutions$ in the existing code.

The "first-search-filter" and "second-search-filter" fields are way to brittle and not flexible at all.

@Jermolene .. What do you think?

[Jermolene]

I think we should merge these suggestions from @ericshulman.

There are some more problems: eg: Several instances of the tag-picker in the same tiddler do not work ... And some more

I think that's a fairly common issue with macros that use state tiddlers. It can be avoided by placing the macro calls within unique transclusions, or by setting up a suitable value for the transclusion variable. At any rate, this is not a new issue introduced by these changes.

[ericshulman]

I would prefer, to be able to call the macro as follows and it should still work.

<<tag-picker tagField:"foo" >>

Did you test it with: \define tag-picker(actions,tagField:"tags",tagFilter:"[get<tagField>enlist-input[]]") as the initial definition? I think it should work and give us both possibilities. Users can provide a different tagField parameter without the need for a tagFilter. .. But it will still be possible to have a tagFilter if they want to.

What do you think?

Your suggestion to use tagFilter:"[get<tagField>enlist-input[]]" may seem reasonable; however, to maintain 100% strict backward-compatibility the default for tagFilter should remain [tags[]]. Otherwise, existing uses of <<tag-picker tagField:"foo">> would have a change in behavior (i.e., showing different droplist contents than it currently does).

Of course, your idea for a completely new field-picker macro would permit a different default filter to be used without any backward-compatibility issues.

[ericshulman]

Several instances of the tag-picker in the same tiddler do not work ...

<<tag-picker tagField:"foo">>
<<tag-picker tagField:"bar">>

This could be addressed in tag-picker-inner() by appending the tagField parameter value to the state tiddler title, like this:

<$vars popid={{{ [[$:/state/popup/tags-auto-complete/]] [<__tagField__>] [<qualify>] +[join[]] }}}>

and then replacing the 3 hard-coded instances of <<qualify "$:/state/popup/tags-auto-complete">> with <<popid>>

[pmario]

Your suggestion to use tagFilter:"[getenlist-input[]]" may seem reasonable; however, to maintain 100% strict backward-compatibility the default for tagFilter should remain [tags[]]. Otherwise, existing uses of <<tag-picker tagField:"foo">> would have a change in behavior (i.e., showing different droplist contents than it currently does).

@Jermolene Eric is right here but I think calling tag picker with <<tag-picker tagField:"foo" >> that shows the "tags" field is completely wrong and imo does not deserve strict backwards compatibility.

strict backwards compatibility shows this --- IMO it should show a list of all possible values of the "foo - field"

@Jermolene ... Just post your decision. I'll implement it that way

[Jermolene]

Thanks @pmario I guess this one of those cases that highlights that bug fixes are necessarily not backwards compatible, an inconvenient truth that we are able to mostly ignore.

Here, I think @pmario makes a compelling case that the current behaviour constitutes a bug, and I'm inclined to agree.

[ericshulman]

I disagree. IMO, there is no compelling need to break 100% backwards-compatibility.

Consider:
Although the current behavior might seem like a bug to some, showing the list of tag values even if the target field is something other than tags (e.g., foo) can actually be a reasonable (and perhaps even common) use-case where someone simply wants to select and store tag values into a field other than the tags field.

Changing this would require that anyone currently using the existing implementation would need to update their wikis to force the list back to containing tag values (i.e., by using tagList:"[tags[]]").

[pmario]

close this one -- has been implemented in a reasonable way since then.