nushell / nu_scripts

A place to share Nushell scripts with each other
MIT License
696 stars 221 forks source link

fix `nu-themes` package #899

Closed amtoine closed 1 month ago

amtoine commented 1 month ago

description

this PR puts back a few themes that have been removed in the previous PR and fixes the commands in the README.

a few questions

@NotTheDr01ds, i have a few questions too :)

fdncred commented 1 month ago

update terminal uses the osc ansi escapes 10, 11, 12 to change the foreground color, background color, and cursor color in terminals that support it.

amtoine commented 1 month ago

update terminal uses the osc ansi escapes 10, 11, 12 to change the foreground color, background color, and cursor color in terminals that support it.

yeah, but when i run the following

source nu-themes/dracula.nu

it immediately changes the theme to Dracula for me

then, the following does not appear to change anything, that's why im not sure i fully understand this command :wink:

use nu-themes/dracula.nu
dracula update terminal
fdncred commented 1 month ago

Correct. This is a bug that @NotTheDr01ds has filed. https://github.com/nushell/nushell/issues/13403

NotTheDr01ds commented 1 month ago

@amtoine Perhaps I need to make the README more clear, but:

Before the change:

After the change:

As for this PR - I'm in the process of updating the old themes (manual) so that they work like the updated ones do. While I'm fine with the "old" ones going in temporarily, I'm working on making sure that they don't get lost again by having their generation be part of the make.nu script.

NotTheDr01ds commented 1 month ago

Correct. This is a bug that ...

Not quite - The behavior that @amtoine describes is correct. If you source the theme, it's fully activated, then a <theme> update terminal isn't going to have any effect, because that code was already run when it was sourced.

amtoine commented 1 month ago

oooooooh, i understand now @NotTheDr01ds !! i was confused about the background changing color on source :scream:

your explanation is much much clearer, thanks a lot :pray: could you mention this in the README?

NotTheDr01ds commented 1 month ago

@amtoine A couple of questions:

amtoine commented 1 month ago

yup, we can do that, thanks @NotTheDr01ds :pray: