[BUG] Inconsistent handling of template variables in button text expressions

[arikorn]

Make sure you're on the latest stable or beta build

• [x] I have tested this on the latest stable or beta release

Is this a bug in companion itself or a module?

• [x] I believe this to be a bug in companion and not a specific module

Is there an existing issue for this?

• [x] I have searched the existing issues

Describe the bug

Buttons using text expressions interpret template variables differently depending on how they're presented. Consider two variables:

$(custom:myVar1) = 123
$(custom:myVar2) = `$(custom:myVar1)` // (probably need to set this with an action)

With these you can set button text two ways:

1. Text expression: `$(custom:myVar1)`. Button shows uninterpreted text: "$(custom:myVar1)"
2. Text expression: $(custom:myVar2). Button shows "123", even though the Variables tab confirms that the value is $(custom:myVar1)

This issue records the "conclusion" of PR #3768, which was presented poorly (by me) and whose proposed solution was found wanting (even by me).

Since the problem is inconsistency, one can solve it either by making case 1 behave like case 2 or by making case 2 behave like case 1. My suggestion had been to resolve custom variables in template literals (at least in button text) in the same way the ${} is resolved, i.e. `text $(module:variable)` is treated the same as `text ${$(module:variable)}` currently is treated. This met with opposition, so another solution to the underlying inconsistency is needed.

Steps To Reproduce

See description, above.

Expected Behavior

The two cases should result in the same text showing up on the button.

Environment (please complete the following information)

- OS: Windows 10
- Browser: Chrome
- Companion Version: latest beta, latest stable.

Additional context

No response

[arikorn]

The following are comments from @Julusian in the above-reference PR, as the pertain to the remaining issue (much of the following, I believe, is referring to my suggestion that `text $(module:variable)` be resolved the same as `text ${$(module:variable)}`):

1. I suppose I am not opposed to this idea, but it needs a bit of refinement. Elsewhere we can handle abc $(custom:$(custom:b)), which this probably should try to too? That might be as simple as using the functionsRaw.parseVariables() when it exists (and when it doesn't, skip this parsing?) This should probably also support doing a \$(asd for when you want the unparsed literal. How to do that without affecting parsing elsewhere will be 'fun'. And ideally, the expression editor should be able to syntax highlight these.
2. It looks like there is an agreement that doing something special for button text is not what we want.
3. Having slept on the parser approach, I am less sold. I am worrying that it will lead to more issues like #2577, where we are overeager in parsing variables, and there is no way to opt out. [but see edit, below]
4. I have another thought; the recursive parsing of variables like we have today:

$(custom:myVar1) = 123
$(custom:myVar2) = `$(custom:myVar1)` // (probably need to set this with an action)

where using $(custom:myVar2) gives 123, is perhaps a relic of a pre expression and especially a pre expression-variable companion. Every use case I can think of for it can be solved with newer features.

So rather than expanding upon that, maybe we should start to phase that out. This could be done in a few steps:

1. Disable this behaviour for all module variables (or is this something that modules will be using?)
2. Make this behaviour opt-in for all custom-variables (enabled for any existing custom-variables/old imports)
3. Write a reminder for ourselves to remove it whenever we get to 5.0

If anyone is relying on this functionality, they can do it explicitly in an expression parseVariables($(custom:myVar2)) would give 123, so if we do break anyones config, we can give guidance on how to fix it. It is wordier/more effort for users, but it avoids quirks in the recursive parsing and removes some unavoidable magic

edit: I keep glossing over that this is intended to work specifically for template strings, assuming it operates on all strings. I can see why it would make sense to support this syntax, but I'm not sure its worth the complexity and diverging from js syntax for this. And it is worth noting that the existing behaviour you refer to of chaining variables is not specific to template strings, but to any string. So if template strings in expressions do this interpolation, why not others?

---

As a side thought; perhaps as part of this, once we support expressions in action/feedback input fields, we should start phasing out any form of the old variable interpolation on these. keep that as something specific for button text and the function in expressions? Idk, a random thought that it is way way too early to act upon, and I am not sure about, so not something to debate here

[arikorn]

... and I (ari) just want to highlight this little comment of Julian's, since it particularly relevant to the issue but gets buried in the breadth of the discussion:

And it is worth noting that the existing behaviour you refer to of chaining variables is not specific to template strings, but to any string. So if template strings in expressions do this interpolation, why not others?

Good point in the first sentence! I'm not sure that the question follows though. I think we're combining multiple issues here, so there's (1) the inconsistent behavior, which as you point out is not limited to template literals and (2) the idea that `${$(module:var)}` is an awkward and unnecessary construction. This latter idea was pretty much resoundingly rejected, as I am apparently the only one who considers is aesthetically unpleasing and repetitive 😀. Then again, maybe I'm missing you're point?