nu_scripts icon indicating copy to clipboard operation
nu_scripts copied to clipboard

Theme preview doesn't show "string" label when a closure is used in `color_config`

Open NotTheDr01ds opened this issue 1 year ago • 6 comments

Trying out an example from @fdncred where:

$env.config.color_config.string = {|x| if $x =~ '^#[a-fA-F\d]+' { $x } else { 'default' } 

If that's set in a theme, the theme preview small shows the closure, but doesn't label it as the string type. All the types with basic (or record) colors show the labels.

On the bright side, none of the current themes use a closure for anything other than date, filesize, and bool, and all of those expand their values instead of showing the closure value. Still, if someone has their color_config set like this, the theme preview will be off. Even if the label was shown properly, attempting to display the closure itself inside the table is going to be problematic from a formatting standpoint. Probably need a different solution here.

NotTheDr01ds avatar Jul 25 '24 23:07 NotTheDr01ds

The data type is a closure, not a string. Maybe I'm not following what you're saying.

fdncred avatar Jul 26 '24 02:07 fdncred

When it's a closure, the label doesn't show. Probably needs a screenshot demo ;-)

Normal:

image

With closure:

image

Notice the "string" label is missing.

NotTheDr01ds avatar Jul 26 '24 07:07 NotTheDr01ds

The other colors that are conditionally set via closures (date, filesize, and bool) are "special-cased" by the preview script, so it displays a "demo" of the closure results. That isn't possible with other closures, of course, since there's no way in advance to know the pattern(s)/condition(s) in the closure.

NotTheDr01ds avatar Jul 26 '24 07:07 NotTheDr01ds

oh, i see what you mean. thanks. i can't remember how i wrote this but is there a way to look at the right hand side and send it to describe to see the data type and then change the output? i think i'll have to look closer at the the script to recall.

fdncred avatar Jul 26 '24 12:07 fdncred

Definitely - That's what the code does now, and why it actually prints the view source of the closure rather than just <closure1234>. The fix for the left-side (string label) is probably pretty easy, but I'm just not sure what to print on the right-side that would be helpful + well formatted.

NotTheDr01ds avatar Jul 26 '24 13:07 NotTheDr01ds

Ah, I just noticed you did special-handle the string closure in preview theme (the uncondensed version). That said, it will only work for your use-case, and not for other conditionals (like the JSON example, if someone did something like that).

NotTheDr01ds avatar Jul 26 '24 13:07 NotTheDr01ds