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

fish_config: Add CLI-based theme selector #8132

Merged
merged 2 commits into from
Jul 14, 2021
Merged

Conversation

faho
Copy link
Member

@faho faho commented Jul 11, 2021

Description

fish_config theme:

  • list to list all available themes (files in the two theme
    directories - either the web_config/themes one or
    ~/.config/fish/themes!)
  • show to show select (or all) themes right in the terminal - this
    starts another fish that reads the theme file and prints the sample
    text, manually colored
  • choose to load a theme now, setting the variables globally
  • save to load a theme and save the variables universally
  • dump to write the current theme in .theme format (to stdout)

Part of #3625.

A screenshot:

Screenshot_20210711_222310

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

Open questions:

  • Should we have separate choose and save commands (like fish_config prompt!)? Should we offer universal variables here, or a conf.d snippet? Should we have save at all?
  • What about pre-defined universal variables? What about saving if there are already globals?
  • Should dumping write to a file?
  • Do we read from ~/.config/fish/themes? If yes, we should also add it to the web version. (at which point the format of .theme files becomes a lot more important!)

(and yes, docs and changelog and tests are still missing)

@faho faho added this to the fish 3.4.0 milestone Jul 11, 2021
@ridiculousfish
Copy link
Member

This is sweet! It looks awesome and works great for me.

Some nits: for fish_config theme show I suggest showing the current theme first, for comparison. Also the list of themes somewhat runs together vertically - perhaps indent the sample text by 2 spaces, and/or add a newline before each title.

Should we have separate choose and save commands (like fish_config prompt!)?

I think we should be consistent with fish_config prompt. If we change one let's change both.

What about pre-defined universal variables? What about saving if there are already globals?

Could we print a warning if there's a global shadowing the uvar? I really do think universal variables will go away eventually, and there will only be globals.

Should dumping write to a file?

I love it how it is: prints to stdout and the user is free to redirect it.

Do we read from ~/.config/fish/themes?

That's a tough question. I suspect themes will end up being fish script, not JSON or other structured formats, so if the theme files are scripts then I don't think we'll paint ourselves into a corner. Do you have plans for how to populate the ~/.config/fish/themes directory, e.g. a separate git repo or something?

@faho
Copy link
Member Author

faho commented Jul 12, 2021

Also the list of themes somewhat runs together vertically - perhaps indent the sample text by 2 spaces, and/or add a newline before each title.

That makes sense!

Could we print a warning if there's a global shadowing the uvar?

We already do!

> set -g fish_user_paths $fish_user_paths
> set -U fish_user_paths $fish_user_paths
set: Universal variable 'fish_user_paths' is shadowed by the global variable of the same name.

I suspect themes will end up being fish script, not JSON or other structured formats

In my solution, they are not scripts. They really are just data. And I think that's quite important.

The syntax is "each line contains whatever comes after set -U , with quoting but without expansions". This isn't really a conscious choice, it was just easy to do. Both the CLI fish_config and the web version then restrict it to variables named fish_*color*.

I'm quite uncomfortable with making them actual scripts, especially if we end up making it available to third parties.

(unfortunately "theme" is used by some third party things to mean "prompt and greeting and such", while this is about a color scheme)

Do you have plans for how to populate the ~/.config/fish/themes directory, e.g. a separate git repo or something?

If we don't have code here, it doesn't really matter. Someone could make a website, we could make a theme of the month contest on fishshell.com, a separate git repo, downloading random themes on startup, ...

@faho
Copy link
Member Author

faho commented Jul 12, 2021

Okay, I moved the printing of the sample text to a new demo subcommand. You can use that to show off your own colorscheme.

This makes it possible to display the current colorscheme first. I'm not quite sold on doing that, because it's too long for a single screen, so you'd need to scroll up to see it again. We could do it last, or we could remove a few themes? Are all of these really used?

Or should we make the lines longer? Move the comment on the first line?

@faho
Copy link
Member Author

faho commented Jul 12, 2021

Alright, tacking the comment onto the end of the first line seems to be a winner:

Screenshot_20210712_210338

At least in the terminal, given how little we can use the width otherwise. It's not a problem in the browser, flexbox will take care of it.

(screenshot taken in gnome terminal this time)

@ridiculousfish
Copy link
Member

The added spacing + comment motion makes it even better, it looks very nice to me.

This makes it possible to display the current colorscheme first. I'm not quite sold on doing that, because it's too long for a single screen, so you'd need to scroll up to see it again. We could do it last, or we could remove a few themes? Are all of these really used?

My typical $LINES is 34 which fits 6.5 themes. I suppose it could be doubled if we used two columns, or even more columns if we shortened some of the text. "bright vixens jump; dozy fowl quack" is just a silly pangram I found.

But we shouldn't allow perfect to be the enemy of the good. I personally don't mind scrolling, and printing a long theme list to stdout seems like a fine place to start.

faho added 2 commits July 14, 2021 18:54
`fish_config theme`:

- `list` to list all available themes (files in the two theme
directories - either the web_config/themes one or
~/.config/fish/themes!)
- `show` to show select (or all) themes right in the terminal - this
starts another fish that reads the theme file and prints the sample
text, manually colored
- `choose` to load a theme *now*, setting the variables globally
- `save` to load a theme and save the variables universally
- `dump` to write the current theme in .theme format (to stdout)
- `demo` to display the current theme
@faho
Copy link
Member Author

faho commented Jul 14, 2021

Alright, I added some docs and completions and made the web version also check ~/.config/fish/themes.

Merging!

@faho faho merged commit 768afad into fish-shell:master Jul 14, 2021
@faho faho deleted the colors branch July 14, 2021 16:56
jorgebucaran added a commit to jorgebucaran/fisher that referenced this pull request Jun 6, 2022
Themes are new in Fish 3.4 (fish-shell/fish-shell/pull/8132).
jorgebucaran added a commit to jorgebucaran/fisher that referenced this pull request Jun 6, 2022
jorgebucaran added a commit to jorgebucaran/fisher that referenced this pull request Jul 3, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants