Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
NotTheDr01ds opened this issue Jul 25, 2024 · 6 comments

Comments

@NotTheDr01ds
Copy link
Contributor

NotTheDr01ds commented Jul 25, 2024

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.

@fdncred
Copy link
Collaborator

fdncred commented Jul 26, 2024

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

@NotTheDr01ds
Copy link
Contributor Author

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
Copy link
Contributor Author

NotTheDr01ds commented Jul 26, 2024

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.

@fdncred
Copy link
Collaborator

fdncred commented Jul 26, 2024

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.

@NotTheDr01ds
Copy link
Contributor Author

NotTheDr01ds commented Jul 26, 2024

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
Copy link
Contributor Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants