txp:variable and the default attribute: bug or expected behaviour?

jakob · 2022-10-21 13:27:25

While trying to make my code more defensive, I wanted to use the default attribute with txp:variable in much the same way as you can with txp:yield when an attribute is not specified, i.e. also like the way that defaults work for css custom properties or sass variables.

The default attribute is not actually listed as an attribute in the docs, but I’m guessing that’s an oversight as it is shown in Example 8.

My use case is to avoid shortcodes breaking that depend on a global variable that might not be set, i.e. to make them more portable:

<txp:variable name="my_variable" default="some default value" />
<!-- potentially also in combination with the output attribute -->

However, I’ve found that the tag throws an Undefined array key "variable_name" error if the variable hasn’t previously been defined. It seems it only assigns the default value when the variable already exists but its value is empty.

My expectation was:

if the variable exists, use it
if it has not been set before, or it is empty, use the specified default

… which felt logical to me, but maybe (probably) I’ve overlooked some other use situation.

Or does that sound logical to you too and what I’m seeing is a “bug”?

If one adds default to the list of attributes like value, add and reset in this line of the variable function – e.g. || isset($atts['default']) – then it works exactly as I expect it to.

——

FWIW: I know you can get around this by doing:

<txp:if_variable name="my_variable" not>
    <txp:variable name="my_variable">some default value</txp:variable>
</txp:if_variable>

or

<txp:if_variable name="my_variable">
    <txp:variable name="my_variable" />
<txp:else />
    some default value
</txp:if_variable>

but neither is as succinct as the one-liner.

etc · 2022-10-21 16:43:49

I hesitate between bug, feature and oversight. Historically, if a variable is not set, <txp:variable name="blah" /> issues a warning and everyone seems to find it handy for debugging. Should we change this behaviour when default is provided? It would then be more difficult to detect misspelled variable names and other things like this.

For the background, as you note, <txp:variable /> has no own default attribute, so the global one is applied. But this happens after the tag has been processed and has issued its warning.

jakob · 2022-10-22 08:07:20

etc wrote #333998:

I hesitate between bug, feature and oversight.

Yes, I thought so :-)

I would respectfully suggest that the active definition of a default implies the intention to cover situations where it has not been defined or has an empty value in preceding code. The regular situation – without default – remains as-is, so if you prefer an error to appear when a variable hasn’t been supplied, or has been mistyped, one can simply omit specifying a default.

I guess the only situation it “invalidates” is when someone thinks they have specified a custom value but have mistyped it and wonder why it’s not having any effect.

etc · 2022-10-23 10:42:54

jakob wrote #334002:

I would respectfully suggest that the active definition of a default implies the intention to cover situations where it has not been defined or has an empty value in preceding code.

This sound logical, but would complicate debugging mistyped variable names:

<txp:variable name="mitsyped" default="something" />

Without the warning, it could be hard to notice that something is the result of the mistyped name.

I’d keep the warning there, but possibly overcharge output attribute, so

<txp:variable name="mitsyped" output />

would issue no warning (regardless default is set or not), and than caveat utilitor.

jakob · 2022-10-23 13:15:45

etc wrote #334008:

I’d keep the warning there, but possibly overcharge output attribute, so

<txp:variable name="mitsyped" output />...

would issue no warning (regardless default is set or not), and than caveat utilitor.

Not sure I understood you correctly. Do you mean by setting output="1" (or without “1”):

the warning would not occur
the tag would output what you provide in default="my output value"
the tag would not set the variable.

Presumably, not setting default or some other attribute such as add or reset results in nothing being output anyway, so no change except for no error message.

Is the code change for that achieved by changing line 5000 of taghandlers.php to:

} elseif ($set === null && !isset($var) && !isset($output)) {

———

Final question. In my use case, I’d been using this construction ahead of a txp:php block and then using global $variable; $variable['my_variable']; to access it in the php code block. I can’t get at the output attribute with that, so am I right in assuming to get the variable in the php code, I’d either need to do:

parse('<txp:variable name="my_variable" default="my default value" output />');

or

variable($name="my_variable", $default="my default value", $output=true);

etc · 2022-10-23 13:41:42

jakob wrote #334009:

Do you mean by setting output="1" (or without “1”):

the warning would not occur

the tag would output what you provide in default="my output value"

the tag would not set the variable.

Presumably, not setting default or some other attribute such as add or reset results in nothing being output anyway, so no change except for no error message.

Yep, a valueless output would just suppress the warning, without setting the variable. We would not even need to add its own default attribute to <txp:variable />, the global one should suffice:

<txp:variable name="mitsyped" /> <!-- issues a warning, empty output -->
<txp:variable name="mitsyped" output /> <!-- no warning, empty output -->
<txp:variable name="mitsyped" output default="something" /> <!-- no warning, default output -->

Is the code change for that achieved by changing line 5000 of taghandlers.php to:

} elseif ($set === null && !isset($var) && !isset($output)) {...

Dunno, you’ll tell me :-)

Final question. In my use case, I’d been using this construction ahead of a txp:php block and then using global $variable; $variable['my_variable']; to access it in the php code block. I can’t get at the output attribute with that, so am I right in assuming to get the variable in the php code, I’d either need to do:

parse('<txp:variable name="my_variable" default="my default value" output />');...

or

variable($name="my_variable", $default="my default value", $output=true);...

I think you’d better use parse here, to set the global default, unless you do it yourself in your code. Otherwise, txp tags signature is

tagfunc(array $atts, $thing = null)

So you could call

variable(array('name'=>"my_variable", 'default'=>"my default value", 'output'=>true));

But this is a bad practice, firstly because of globals, and then because tag names do not always match their php functions. A better way is

processTag('variable', array('name'=>"my_variable", 'default'=>"my default value", 'output'=>true));

This should handle the global attributes too.

jakob · 2022-10-23 15:50:19

} elseif ($set === null && !isset($var) && !isset($output)) {

etc wrote #334010:

Dunno, you’ll tell me :-)

:-) Yes, the addition of && !isset($output) seems to be all that’s needed. I’ll do a mini-PR for that.

I think you’d better use parse here … otherwise, txp tags signature is

Thank you for clarifying that too. I had a tag-in-tag situation which conflicts with the quotes, so I escaped the one’s within and it seemed to work, e.g.

$assets_path = parse('<txp:variable name="assets_path" default=\'/themes/<txp:page_url type="theme" />/assets/\' output />');

Textpattern CMS

Textpattern CMS support forum

#1 2022-10-21 13:27:25