Skip to content

Conversation

@faho
Copy link
Member

@faho faho commented Jul 9, 2021

Description

As we know, fish_config currently is the only place you can see the colorschemes. There's no way inside the shell to access them, which is profoundly weird.

So, what we do is store the themes in ".theme" files instead. These are a fishscript-influenced key-value format, basically what set -L prints. Essentially, every line is either a comment (including a metadata comment) starting with "#", empty, or the arguments for a set call from the variable name onwards:

# name: ayu Light
# url: 'https://github.com/dempfi/ayu'
# preferred_background: FAFAFA

fish_color_normal 575F66
fish_color_command 55B4D4
fish_color_quote 86B300
fish_color_redirection A37ACC
fish_color_end ED9366
fish_color_error F51818
fish_color_param 575F66
fish_color_comment ABB0B6
fish_color_match F07171
fish_color_selection --background=FF9940
fish_color_search_match --background=FF9940
fish_color_history_current --bold
fish_color_operator FF9940
fish_color_escape 4CBF99
fish_color_cwd 399EE6
fish_color_cwd_root red
fish_color_valid_path --underline
fish_color_autosuggestion 8A9199
fish_color_user brgreen
fish_color_host normal
fish_color_cancel -r
fish_pager_color_description B3A06D yellow
fish_pager_color_completion normal
fish_pager_color_prefix normal --bold --underline
fish_pager_color_progress brwhite --background=cyan

This makes it easy to read from both webconfig.py, and a fish script (to be written and integrated into fish_config as fish_config theme).

Little should currently change about the fish_config browser interface, except for that the themes have been reordered. They're now in alphabetical, case-insensitive order except for the current and default themes, which are first.

The motivation for all of this is that it allows us to:

  • Add a terminal UI for picking themes
  • Add a way to add themes for third-parties (this is safe because there is no code here, unlike prompts)
  • Make it easier to add themes to fish proper (by not requiring weird javascript - we could even make a theme dumper!)

Part of #3625.

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

faho added 6 commits July 9, 2021 18:26
This used to pass each color in a separate url-encoded request, which is
just wasteful.

Also it passed separate parameters for modifiers like bold and
underlined, but never gave them actual values. Instead the color is
passed as one string.

So we just use json, and then iterate over it server-side.
These are simple

var val [val val]

files. Basically the bit in `set -g fish_color_escape 86c1b9` after
the `set -g `. Since we're not going to `source` them, however,
arbitrary code and expansions are unsupported.

Also comments and such don't currently work.

This allows them to be easily readable both from webconfig (next
commit) and the shell (later).
Following a "#" comment. Also empty lines, so we have to deal with them.
This readds the preferred background and url, all read from "# url:"
and "# preferred_background:" comments in the .theme file.
This still keeps the Nord and Solarized palettes because we use the
backgrounds in the sample background list.
@faho faho added this to the fish 3.4.0 milestone Jul 9, 2021
@faho
Copy link
Member Author

faho commented Jul 9, 2021

Note: I extracted the themes by setting them from webconfig (I hacked it so it only printed the set commands instead of executing them). This means they've lost the relation to the palette, and now it's all just a list of variables.

I don't see how we can keep a non-code format without losing that, and that's imperative to make it usable from python and fish, and to allow others to add things (imagine a ~/.config/fish/themes directory that fish_config just picks up! imagine a theme of the month on fishshell.com!)

This has cheesy pattern matching that I'm not entirely sure adds
anything?

Surely if we add something to share/web_config that should be
installed *by default*?

Anyway, let's just add .theme to it
Copy link
Member

@zanchey zanchey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works well in my testing, and it's a great idea.

I'm not able to give much comment on the JS.

Can I suggest running black over the Python source, either before and after your changes or just after?

@faho
Copy link
Member Author

faho commented Jul 11, 2021

So I'm close on the CLI. Should I add that to this PR or merge this (which doesn't really do a lot that's visible) and open another?

@krobelus
Copy link
Contributor

What if we changed the theme format to (retricted) fish

so set fish_color_normal 575F66 instead of fish_color_normal 575F66?

then it would be fairly obvious how to load the theme - just source the file.
The canonical way would still be through fish_config (also CLI), which will offer proper completions.
Or would that just cause confusion, because people might try to use fish features in their themes, which won't work.
So maybe a new format brings the least trouble.
As long as fish_config can load arbitrary files as themes, so users can experiment, it's probably good enough.

This seems good to merge (I'd run the formatter though).


If the addition + removal of the themes were in the same commit, it would be slightly easier to trace the evolution of the themes. But I didn't see a good way to do that while keeping the commit structure.

@faho
Copy link
Member Author

faho commented Jul 11, 2021

What if we changed the theme format to (retricted) fish

That just means people start running source on it, and start putting unrestricted fish in it.

Also that makes it harder to pick the scope of the variable - in the current CLI I run set -g or set -U depending on the arguments, and once you add a set here you need to weirdly insert the scope in the middle.

Overall, I like having a clear line between a .theme and a .fish.

An alternative would of course be picking an actual serialization format, but most of them are more annoying to read (and especially more annoying to generate from the shell).

Possibly var=val, but that makes dealing with lists annoying - how do you store a variable set like set -U fish_color_status '--background=red' 'white'? Do you just go fish_color_status=--background=red white or is it fish_color_status='--background=red white' and you have to tokenize again?

@faho
Copy link
Member Author

faho commented Jul 11, 2021

Alright, black doesn't really change a lot, so I'm gonna merge this now and run a full formatting pass later.

This also allows me to open another PR for the CLI bit of this, I have some open questions.

@faho faho merged commit c5d4f26 into fish-shell:master Jul 11, 2021
krobelus added a commit to krobelus/fish-shell that referenced this pull request Dec 12, 2021
I used the command from fish-shell#8092 to list issues/PRs with missing changelog entries,
and went through most of them.

Each issue gets three lines
- subject
- URL
- verdict

If the verdict ends with ", ignoring", I added it the the ignored list in the changelog.

The issues are grouped by verdict, with the interesting/leftover ones
on top (obviously no need to double check *everything*).

The "gh" script is already a quantum leap we can still find better
ways to share the changelog burden.
I noticed that there are many minor updates that can probably be
ignored.  Filtering them out doesn't take much time but it adds up,
especially if it's a single person doing it.

---

Issue/PR title: Make --no-config mode more comfortable
fish-shell#8493
in-flight

Issue/PR title: Debian fish binary package difficult to install
fish-shell#7845
TODO

Issue/PR title: Work around `setpgid` error on older Apple platforms
fish-shell#8153
this extends 7474, which wasn't listed either, we should probably mention it?

Issue/PR title: Abbr -q return status inconsistent
fish-shell#8431
fixes return status of "abbr -q", should we mention this?

Issue/PR title: math: (n n): incorrect error
fish-shell#8511
improved error output, which is very nice but too minor?

Issue/PR title: Fish autocomplete error on iOS procursus
fish-shell#8205
niche fix, ignoring

Issue/PR title: Fix `fish_key_reader` wrapper check
fish-shell#8271
minor update to not create a harmless alias for fish_key_reader, ignoring

Issue/PR title: funced dosn't like backslash escapes in function names
fish-shell#8289
minor escaping fix, ignoring

Issue/PR title: Hide whatis database building from the user
fish-shell#8310
not something many users would notice in the first place, ignoring

Issue/PR title: Duplicated "Type 'help argparse' for related documentation" for argparse
fish-shell#8368
minor update to error message, ignoring

Issue/PR title: Variable highlight color does not span lines
fish-shell#8444
very obscure fix, ignoring

---

Issue/PR title: Add --function to `read`
fish-shell#8295
added to existing entry (565)

Issue/PR title: Added completions for ethtool
fish-shell#8283
added to existing entry

Issue/PR title: Add dart completion
fish-shell#8315
added to existing entry

Issue/PR title: Add common lisp completions(sbcl/roswell)
fish-shell#8330
added to existing entry

Issue/PR title: Fix st issue with shift+tab
fish-shell#8354
added to existing entry (8352)

Issue/PR title: Support vi-mode cursors in Foot Terminal 
fish-shell#8391
added to existing entry (8167)

Issue/PR title: Completions pager should redraw if the subbed completion wraps/unwraps the line
fish-shell#8405
added to existing entry (8509)

---

Issue/PR title: Binding escape as user binding breaks escape sequence bindings (arrows, etc)
fish-shell#8428
added new entry

Issue/PR title: Windows "color" command completion
fish-shell#8483
added new entry

Issue/PR title: Doesn't build when using netbsd curses on Linux
fish-shell#8087
added new entry

Issue/PR title: Don't override linker
fish-shell#8152
added new entry

Issue/PR title: Add completions for `git-sizer`
fish-shell#8156
added new entry

Issue/PR title: `d3ceba107e88b6c6e1a0358ebcb30366aeef653f` causes issues with repainting multi-line prompt
fish-shell#8163
added new entry

Issue/PR title: Builtin math ncr can be extremely slow
fish-shell#8170
added new entry

Issue/PR title: Completion sometimes missing the last token
fish-shell#8175
added new entry

Issue/PR title: `set -S` should mark read-only variables
fish-shell#8179
added new entry

Issue/PR title: Errors when trying to autocomplete (invalid) UTF-8 escapes
fish-shell#8195
added new entry

Issue/PR title: Always use LC_NUMERIC=C internally
fish-shell#8204
added new entry

Issue/PR title: Slow interaction between backgrounding, universal variables, and repainting
fish-shell#8209
added new entry

Issue/PR title: Unsetting `$fish_emoji_width` doesn't clear the cached width
fish-shell#8274
added new entry

Issue/PR title: If prompt ends in an empty line, the commandline is inserted at the width of the line before
fish-shell#8298
added new entry

Issue/PR title: assertion normal_exited() failed related to paged builtin help
fish-shell#8308
added new entry

Issue/PR title: colors don't kick in for ls on macOS Big Sur, Monterey (and maybe FreeBSD)
fish-shell#8309
added new entry

Issue/PR title: Adds sub-command clear-session to history command. Issue fish-shell#5791
fish-shell#8337
added new entry (as 5791)

Issue/PR title: Display local branches before unique remote branches in git completion
fish-shell#8338
added new entry

Issue/PR title: Fix delete-key in st
fish-shell#8352
added new entry

Issue/PR title: sigsegv on set --show variable (when LANG is set to fr_FR.utf8)
fish-shell#8358
added new entry

Issue/PR title: Add clasp completion
fish-shell#8373
added new entry

Issue/PR title: `cargo run --example` completions break with nested example directories
fish-shell#8429
added new entry

Issue/PR title: argparse completions
fish-shell#8434
added new entry

Issue/PR title: Stop linking to StackOverflow
fish-shell#8495
added new entry

Issue/PR title: fish_key_reader ^C warning isn't right
fish-shell#8510
added new entry

Issue/PR title: Use `--almost-all` in `la` function
fish-shell#8519
added new entry

---

Issue/PR title: improve the experience of using fish over mosh
fish-shell#1363
listed as 8376, ignoring

Issue/PR title: incomplete man page completions
fish-shell#8305
listed as 8309, ignoring

Issue/PR title: Support "$(cmd)" command substitution without line splitting
fish-shell#8059
listed as 159, ignoring

Issue/PR title: fish_config: Read colorschemes from .theme files
fish-shell#8127
listed as 8132, ignoring

Issue/PR title: funced: edit the whole file, not just the function definition
fish-shell#8130
listed as 391, ignoring

Issue/PR title: builtin cd: print error about broken symlink
fish-shell#8270
listed as 8264, ignoring

Issue/PR title: fix man completion for BSD's mandoc
fish-shell#8306
listed as 8305, ignoring

Issue/PR title: Don't escape tildes that come from custom completions
fish-shell#8441
listed as 8441, ignoring

Issue/PR title: Use `cargo run --example` to get list of examples
fish-shell#8446
listed as 8429, ignoring

---

Issue/PR title: Node completion: add v8 sparkplug option
fish-shell#8118
update to existing completions, ignoring

Issue/PR title: Add zypper subcommands completion
fish-shell#8183
update to existing completions, ignoring

Issue/PR title: completion nmap: suppress warning when local scripts folder exists
fish-shell#8184
update to existing completions, ignoring

Issue/PR title: add missing `git commit` completions
fish-shell#8191
update to existing completions, ignoring

Issue/PR title: Updated ping completions
fish-shell#8192
update to existing completions, ignoring

Issue/PR title: Add `--function` to `set` completion
fish-shell#8202
update to existing completions, ignoring

Issue/PR title: completion: support `--no` prefixes for mpv flag options
fish-shell#8219
update to existing completions, ignoring

Issue/PR title: complete "mpc load"
fish-shell#8241
update to existing completions, ignoring

Issue/PR title: Add and fix completions for new options
fish-shell#8243
update to existing completions, ignoring

Issue/PR title: Fix completions/ls.fish
fish-shell#8249
update to existing completions, ignoring

Issue/PR title: Fix completions/coredumpctl.fish and add new complete
fish-shell#8256
update to existing completions, ignoring

Issue/PR title: completions/git: Handle "1 .T" & "1 AT" files
fish-shell#8311
update to existing completions, ignoring

Issue/PR title: completions/xbps-query: add missing `-p` completions
fish-shell#8323
update to existing completions, ignoring

Issue/PR title: Update ldapsearch.fish
fish-shell#8326
update to existing completions, ignoring

Issue/PR title: small fix completions/duply.fish
fish-shell#8327
update to existing completions, ignoring

Issue/PR title: Update ip.fish
fish-shell#8334
update to existing completions, ignoring

Issue/PR title: Fix ant completion
fish-shell#8344
update to existing completions, ignoring

Issue/PR title: Update dmesg completions
fish-shell#8365
update to existing completions, ignoring

Issue/PR title: No hints for -g|--global and -U|--universal flags for abbr command
fish-shell#8367
update to existing completions, ignoring

Issue/PR title: Updated systemd-analyze completions
fish-shell#8381
update to existing completions, ignoring

Issue/PR title: vmctl completion function call needs to be quoted
fish-shell#8406
update to existing completions, ignoring

Issue/PR title: pabcnetcclear command completion update
fish-shell#8480
update to existing completions, ignoring

---

Issue/PR title: document `--no-config`
fish-shell#8176
doc update, ignoring

Issue/PR title: Theme demo needs to be adjusted so that only unmatched quote is an error
fish-shell#8260
doc update, ignoring

Issue/PR title: no error about wrong >>? redirection operator
fish-shell#8380
doc update, ignoring

Issue/PR title: set -l works outside of command block
fish-shell#8385
doc update, ignoring

Issue/PR title: Some enhancements to "for" and "while" loop pages
fish-shell#8409
doc update, ignoring

Issue/PR title: Html docs: Remove link underlines again?
fish-shell#8439
doc update, ignoring

Issue/PR title: Old-style options support "=" assignment operator in complete builtin
fish-shell#8457
doc update, ignoring

Issue/PR title: Document prompt_hostname
fish-shell#8522
doc update, ignoring

---

Issue/PR title: edit_command_buffer: use "command" to ignore any functions with the same name
fish-shell#8221
only helps broken systems, ignoring

Issue/PR title: Prepend command to cat
fish-shell#8287
only helps broken systems, ignoring

Issue/PR title: Make less version check compatible with older Fish
fish-shell#8299
only helps broken systems, ignoring

Issue/PR title: Skip leading `command` in `__fish_man_page`
fish-shell#8447
only helps broken systems, ignoring

Issue/PR title: fish_config doesn't work without curses module
fish-shell#8487
only helps broken systems, ignoring

---

Issue/PR title: fix 'socket file name too long' error
fish-shell#8128
test fix with long tempdirs (macOS), not really user-visible, ignoring

Issue/PR title: Give tests a more generic name
fish-shell#8449
not user-visible, ignoring

Issue/PR title: string tests sometimes failing on macOS (Github Actions)
fish-shell#8353
not user-visible, ignoring

Issue/PR title: history merge test fails on OpenBSD
fish-shell#6477
not user-visible, ignoring

---

Issue/PR title: Obtain Deno completions from itself
fish-shell#8471
update to an unreleased feature (7138), ignoring

Issue/PR title: `string length --visible` performance
fish-shell#8253
update to an unreleased feature, ignoring

Issue/PR title: Backspace character is ignored when calculating string widths
fish-shell#8277
update to an unreleased feature, ignoring

Issue/PR title: `fish_config choose` leaves previous right prompt in place
fish-shell#8314
update to an unreleased feature, ignoring

Issue/PR title: parenthesis characters outer of $(command substitution) in string cause error
fish-shell#8394
update to an unreleased feature, ignoring

Issue/PR title: Parser bug with command substitutions in strings inside parenthesis
fish-shell#8500
update to an unreleased feature, ignoring

Issue/PR title: fish_config: silently doesn't set color schemes.
fish-shell#8419
regression, not in any release, ignoring

Issue/PR title: :program: in sphinx doesn't link
fish-shell#8438
regression, not in any release, ignoring

Issue/PR title: __fish_seen_argument.fish throws exception when autocompleting
fish-shell#8478
regression, not in any release, ignoring

---

Issue/PR title: Fix typo in abbr docs
fish-shell#8280
typofix, ignoring

Issue/PR title: Fix typo in `set_colors` command documentation
fish-shell#8321
typofix, ignoring

Issue/PR title: Typo funcions -> functions
fish-shell#8257
typofix, ignoring

---

Issue/PR title: remove make_pair
fish-shell#8206
no behavior change, ignoring

Issue/PR title: replace push_back with emplate_back
fish-shell#8222
no behavior change, ignoring

Issue/PR title: clang-tidy: remove pointless virtual
fish-shell#8224
no behavior change, ignoring

Issue/PR title: change value to rvalue reference
fish-shell#8227
no behavior change, ignoring

Issue/PR title: convert const ref to rvalue ref
fish-shell#8228
no behavior change, ignoring

Issue/PR title: clang-tidy: use for range loops
fish-shell#8229
no behavior change, ignoring

Issue/PR title: fix deleted constructors
fish-shell#8230
nno behavior change, ignoring

Issue/PR title: clang-tidy: const reference conversions
fish-shell#8231
no behavior change, ignoring

Issue/PR title: clang-tidy: simplify two bool returns
fish-shell#8235
no behavior change, ignoring

Issue/PR title: clang-tidy: replace size comparisons with empty
fish-shell#8237
no behavior change, ignoring

Issue/PR title: clang-tidy: replace NULL with nullptr
fish-shell#8239
no behavior change, ignoring

Issue/PR title: add constexpr
fish-shell#8252
no behavior change, ignoring

Issue/PR title: __fish_seen_subcommand_from and __fish_seen_argument update
fish-shell#8430
no behavior change (apart from a regression that's fixed), ignoring

Issue/PR title: Run fish_indent on all non-test .fish files
fish-shell#8476
no behavior change, ignoring

Issue/PR title: Use test command instead of bracket command
fish-shell#8477
no behavior change, ignoring

Issue/PR title: Fix code scanning alert - Wrong type of arguments to formatting function
fish-shell#8521
no behavior change, ignoring

Issue/PR title: clang-tidy: replace push_back with emplace_back
fish-shell#8236
no behavior change, ignoring

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

3 participants