Simplify theme usage

[NotTheDr01ds]

• Primary change: Simplifies theme usage by adding an export-env to each theme so that $env.config.color_config is automatically set when importing the module.

  Rather than the old:

  use nu-themes/nushell-dark.nu
  $env.config.color_config = (nushell-dark)

  It's now a single-step:

  use nu-themes/nushell-dark.nu

• Updates make.nu to create the theme files (uses the previously added str dedent for clean output).

• A couple (literally, 2) of the remote themes did not have all necessary colors and failed. Updated make.nu to continue on error, but report the failures.

• Updated README.md

[fdncred]

Nice! Do all the preview-*.nu scripts still work?

[NotTheDr01ds]

Nice! Do all the preview-*.nu scripts still work?

I'm ... uh, not sure! What and where are they?

[NotTheDr01ds]

Also not sure why CI is failing. Is toolkit check pr really working? I took a look at it yesterday and it looks fairly broken, but I haven't gone too far into it. In this case, it looks like it's expecting make.nu to be a module, but it fails because it's a script.

[fdncred]

What and where are they?

In the themes directory. It's what I used to generate all the screenshots.

[fdncred]

I'm not worried by the CI, it doesn't work very well.

[NotTheDr01ds]

What and where are they?

In the themes directory. It's what I used to generate all the screenshots.

Ah yeah - Just found theme a few seconds ago. Probably needs updating - Brb ;-)

[NotTheDr01ds]

I think it's a simple change, but:

1. I can't test it at the moment under Linux where I'm working on the PR (I'll get something set up)
2. It looks like there's at least one issue that predates this PR -- Your preview_theme_* assumes the directory is named themes/themes, but the repo directory is themes/nu-themes.

As I work on this, do you have a preference on what the directory should actually be named? I'm good with either one, but it seems like it's been changed at some point.

[fdncred]

I'm fine with the current name for folders but I've always thought the preview scripts belonged in a different folder since they're not themes. I just left them in there because it was just easier to generate the theme images.

I wrote those on Windows, but I wouldn't be opposed to having a version that works on Linux or MacOS.

[NotTheDr01ds]

I'm going to work on updating it so it will at least run on WSL and Windows native, but I don't have a Mac or pure-Linux (even virtual machine) at the moment.

I'm thinking we should at least move the screenshot code to a closure that can be run based on the host system. Then people can add screenshot code in the future for other platforms.

Anyway, I'm going to do some refactoring on it, and I'll commit more later - Probably tomorrow.

[fdncred]

I was playing with this PR this morning. I'm not a super fan of having double the files with the *_colors.nu files. I also couldn't try any of the themes without being in the nu-themes folder.

Maybe I'm just looking at it too early and it's not done yet?

[NotTheDr01ds]

As I was working through your preview scripts, I noticed that the main README and process for setting a theme has always been ... very wrong.

The modules here only set the $env.config.color_config to the theme colors. That's great, but the modules failed to set the terminal foreground, background, and cursor color. You do that correctly in your preview scripts, of course.

The latest commits fix this. While I could, of course, set the terminal colors and color_config in the same module, there's benefit to having this separated. I personally prefer the color_config but don't typically want to use the terminal colors. The new scheme creates two modules for each theme - One which has the definitions and no other side-effects, and the other (main) that actually changes the colors for both Nushell and the terminal.

Okay, now back to working on the preview scripts - They should be much easier now.

[NotTheDr01ds]

Maybe I'm just looking at it too early and it's not done yet?

Ah yes, definitely wasn't done yet (and still isn't). But it should have been "workable" in that state.

I was playing with this PR this morning. I'm not a super fan of having double the files with the *_colors.nu files. I also couldn't try any of the themes without being in the nu-themes folder.

I've tried every which way that I can to have a module that has the color definitions and can be use'd without also setting the colors, but I don't think it's possible. Either a module export-env's or not - There's no way to "optionally" import the module and have just the definitions loaded.

For most users, they'll just use nu-themes/<theme>.nu like always. That will now set the colors of the terminal and Nushell.

Users who prefer the older scheme (which I do) can load the definitions separately with use nu-themes/theme-colors/<theme>.nu * and have access to the definitions.

[fdncred]

I really don't want to have double the files for themes. It just makes them way more cumbersome.

[NotTheDr01ds]

Does it help that the files are now in a separate folder?

Otherwise, I think we're going to have to just throw away the PR (and that's okay), since we can't obtain the color definitions any other way without also changing the colors. That's not how it worked before, and I really don't want to lose the ability to set the $env.config.color_config without also changing the terminal colors.

I can put in an enhancement request for a way to do this in a single module, and then we can revisit if that gets implemented.

Or, we can go with the second module and fix it if that enhancement gets implemented.

I'm okay with any path.

[NotTheDr01ds]

Hmmm - One more thing to try to get them in a single file ... Probably won't work, but crossing my fingers.

[NotTheDr01ds]

Hmmm - One more thing to try to get them in a single file ... Probably won't work, but crossing my fingers.

Well, it didn't, but eventually I came up with something that allowed me to get rid of the duplicate module files.

@fdncred Thanks for pushing back on this. I didn't like it either, but I just hadn't found a good alternative. I think I'm happy with the latest iteration, and I hope you will as well.

The interesting thing about where I ended up is that it is fully backwards-compatible with the older version. In other words, this still works exactly the same:

> use ./themes/nu-themes/<theme_name>.nu
> $env.config.color_config = (<theme_name>)

But that still leaves the terminal colors untouched. If the user wants to set everything in one step, they will source the file instead of importing the module.

source ./themes/nu-themes/<theme_name>.nu

Note: I was trying to have two different submodules in the same file, but there seems to be a Nushell bug that prevents the export-env from running when importing a submodule, so that didn't work. I'll write that up, but in the meantime, I think the source option to activate works just as well - Happy to hear your thoughts.

Side-note: The 3024 theme is problematic, since the parser will always see that as a number rather than a command or module name. However, I'm fairly confident that bug existed already. We could special case it by changing the name to 3024-theme or something like that (manually, in make.nu), but I haven't worried about that at this point -- Is there a way to call a command that is purely a number?

[NotTheDr01ds]

Next up, if @fdncred is okay with this version, is to work on the preview scripts. They should still work on your system since the module-usage is the same once again. But I still hope to make them more universal.

[fdncred]

The 3024 theme is problematic, since the parser will always see that as a number rather than a command or module name.

Indeed, it's always been like that. That's why there was a 3024r.nu 3024-day.nu and 3024-night.nu.

Yup, I'm fine with this. Nice touch maintaining backwards compatibility.

I commented on your issue already about the other bug you found.

Let's go!

[fdncred]

Darn it! I just noticed this. These need to be restored, I think. Thoughts?

 delete mode 100644 themes/nu-themes/catppuccin-latte.nu
 delete mode 100644 themes/nu-themes/catppuccin-mocha.nu
 delete mode 100644 themes/nu-themes/nushell-dark.nu
 delete mode 100644 themes/nu-themes/nushell-light.nu
 delete mode 100644 themes/nu-themes/tokyo-moon.nu

I don't want to lose these additional themes.

[NotTheDr01ds]

@fdncred I'll probably figure this out about the same time you answer (seems to commonly be the case ;-), but were those manually defined or sourced from somewhere else?

[fdncred]

No worries. Manually defined from contributors.

[NotTheDr01ds]

Ok, I'll add them back in, and hopefully in a way that they won't get lost again since they'll be generated by make.nu in the future.

[NotTheDr01ds]

@fdncred Question on the preview-* scripts ...

I'm guessing that there was some limitation when you wrote it that required generation of a separate preview-screenshot-script.nu, but I think I can rewrite the main script to just cycle through the themes without requiring a secondary script. Any objection there or something else that I might be missing?

[fdncred]

No objections if it works. It's been a while since I wrote that so I don't remember exactly why I had to do that.