Theme switcher v2 - #6083
Draft
Makihataima-Ken wants to merge 32 commits into
Draft
Theme switcher v2#6083Makihataima-Ken wants to merge 32 commits into
Makihataima-Ken wants to merge 32 commits into
Conversation
yaml.Unmarshal leaves a slice alone when the YAML has no key for it. So when a config file that is loaded after another one (a per-repo config file, or a later file in LG_CONFIG_FILE) has no customCommands key, the custom commands collected so far are still in place after unmarshalling it, and they then get appended to themselves. Every such file doubles the list, so each custom command is bound to its key at least twice. The keybindings menu hides this, because it drops duplicates.
Start each config file from an empty list of custom commands, so that a file without a customCommands key contributes none. The commands of later files still come first, so that they keep taking precedence over those of earlier files that use the same key. Selecting a theme is about to add another config layer, and a theme file never has custom commands. Without this fix, selecting a theme would double the custom commands of everyone who has any.
A custom command prompt with the branches suggestions preset filters its suggestions on a worker whenever the input changes, and it colored each matching branch name there too. Coloring a branch name reads the branch color patterns and the theme's default text color, which the UI thread replaces whenever it applies the user config, for example when lazygit regains focus after a config file was edited. Nothing ordered the worker's reads against those writes. Color all branch names once, on the UI thread, when the prompt is created, which is also where the branch names themselves are read, and let the worker only filter the finished suggestions. Opening the prompt doesn't get slower: it asks for the suggestions of the empty input on the UI thread right away, and that colored every branch before too. The filter mode is now read up front as well, as the presets built on FilterFunc already do it, so the worker reads nothing that the UI thread writes. A config change made while the prompt is open shows up the next time it opens. The other suggestions funcs produce plain labels, so none of them reads colors on the worker. There is no commit that demonstrates the race first, because no assertion can observe it; only the race detector can. The new test changes the colors after creating the func and checks that the labels keep the colors they were created with.
Git commands run on worker goroutines, and they write themselves to the command log. LogCommand picked the text color from the theme before it bounced the write to the UI thread, so the worker read the theme's default text color. The UI thread replaces that color whenever it applies the user config, for example when lazygit regains focus after a config file was edited, and nothing ordered the worker's read against that write. Pick the color in the bounce instead, next to the other writes to the command log. Removing a custom patch from its commit, moving it to another commit or to the index, pulling it out into a new commit and discarding lines from a commit all build the patch to apply on a worker, while the rebase runs. When the patch holds part of a file, the plain formatting used for that read the default text color for every header line, hunk header and context line, although plain output ignores all styles. Now only the colored formatting for views reads the theme, and that runs on the UI thread. No other code reads the theme's variables off the UI thread. There is no commit that demonstrates these races first, because no assertion can observe them; only the race detector can, and only when a config is applied while a git command runs. No integration test can set that up: the test driver waits until lazygit is idle after the focus event that makes lazygit reload a changed config file.
The output of the commands that fill the main and secondary views, and of the git commands that stream into the command log, is written into those views on a worker goroutine. When gocui can't parse an escape sequence in that output, it writes the characters of the sequence as text, and it gave their cells the view's foreground and background colors. The UI thread sets those colors whenever it applies the user config, for example when lazygit regains focus after a config file was edited, and nothing ordered the worker's read against that write. Give these cells the default colors instead. View.draw replaces the default colors with the view's own, as it does for all other text that has no colors of its own, so the characters look the same as before. Like that other text, they now also follow the view's colors when those change after the characters were written. There is no commit that demonstrates the race first, because only the race detector can see it, and only when a config is applied while a broken escape sequence is written.
The status manager decides which of its waiting statuses and toasts the status line at the bottom left shows, and nothing tests it yet. Pin down the rules that must stay before changing how toasts and waiting statuses compete for the line: a waiting status gets a spinner and a toast doesn't, the newest of several waiting statuses or of several toasts wins, a toast shows ahead of an older waiting status, error toasts are red, and a waiting status shows again once the toast in front of it has expired.
The status line always shows the newest status, so a waiting status that starts while a toast is showing takes the line from it right away. The toast expires after two seconds, or four for an error, while the waiting status stays for as long as its operation runs. If that takes longer than the toast's time, the toast is never seen again. This happens, for example, to a toast shown shortly before the periodic background fetch shows "Fetching".
A toast is only shown for a few seconds, so a waiting status that takes the line from it can make it disappear for good. A waiting status, on the other hand, is still there after the toast has expired and shows again then, so letting the newest toast go ahead of all waiting statuses loses nothing. Among toasts, and among waiting statuses, the newest still wins. This matters in particular at startup, because the initial fetch shows its "Fetching" status as soon as lazygit is up. A toast raised while lazygit starts up, such as the one about to report a selected theme that can't be loaded, would stay hidden until the fetch is done, and by then it has often expired.
Applying the theme in onUserConfigLoaded gives the focused view the frame color of an active view, which is the wrong one while a search or filter is active there. When lazygit regains focus after a config file was edited, a focused side panel gets the searching color back because reloadSidePanels focuses it again, but other views, such as a filtered menu, kept the color of an active view until the focus moved. Render the search status of the current view again after a reload on focus, as focusing a view does. The integration test driver can't check frame colors, so there is no test for this.
LogCommand styled every command that could be run on the command line with the theme's default text color. The command log is only ever appended to, so after a switch to a theme with a different default text color, the commands logged before it kept the old color for the rest of the session. With a light theme's dark text color, for example, they become hard to read after switching to a dark theme. An edit of config.yml that changes the default text color left them behind in the same way. Write these commands without a style instead. gocui draws unstyled text in the view's text color, which is the same color and is updated whenever the theme is applied, so the whole log follows the theme without being rendered again, and LogCommand no longer reads the theme.
The commit graph caches the strings it renders for RGB styles, keyed by the address of the style object. Author colors are such styles, and every time they are set, which happens when the config is reloaded and when the terminal switches between a light and a dark background, each author gets a new style object, even if its color stays the same. So the entries of the old styles are never looked up again, but nothing removes them either, and the cache grows by a full set of entries with every color change. The cached pipe sets are already dropped when the author colors change, because they hold the old styles. Drop this cache at the same point, so that it only holds entries for the styles that are in use. Emptying the cache changes nothing on screen, so no integration test can see it; a unit test of the graph package checks the reset directly.
The GitHub pull request cache lives in the folder of state.yml, and the file that remembers the selected theme is about to live there too. Both have to find that folder through state.yml itself: stateFilePath finds a file in a legacy config folder only if that file already exists there, so asking it for a file that doesn't exist yet could put that file in a different folder than state.yml.
Theme files are about to be converted from the layout that has gui.authorColors, gui.branchColorPatterns or gui.branchColors directly in gui, by the same steps that migrate these keys in config files, but without the other migrations: those reject YAML aliases, which theme files may use. The worktree keybinding is now moved before these steps instead of between them. This only changes the order of the list of changes that the user sees when a config file needs both migrations.
A merge key (<<) in a map of branch color patterns becomes a pattern of its own, with no color, and the patterns that it merges are missing from the list, although yaml has decoded their colors.
UnmarshalYAML built the list of patterns from the keys as they are written, so a merge key (<<) became a pattern with no color, and the patterns that it merged were missing, although yaml had decoded their colors. The merged patterns now take the place of the merge key, except for those that the map sets itself, which keep their own place. Of the maps in a list, the first one that has a pattern decides its place, just as yaml takes its color from there. In config.yml this happens only with a map written in place after the merge key, because the migrations of config.yml reject aliases, but theme files are about to be loaded without these migrations, and aliases are the usual way to use merge keys.
Switching between color themes means editing config.yml, or juggling a list of files in LG_CONFIG_FILE. A theme is now an overlay file in the themes folder of the config dir, and the name of the selected one is read from selected_theme.yml whenever the config is reloaded for a repo, which happens at startup and on every repo switch. The name has a file of its own, next to state.yml, because every running lazygit rewrites state.yml from memory, which would undo a selection made in another instance. The theme comes after the global config files, so that selecting it overrides the colors in config.yml, but before the per-repo files, so that a repo can still have colors of its own. It is never part of the config that NewAppConfig loads, so SaveGlobalUserConfig can't bake it into config.yml in integration tests. The gui.darkTheme and gui.lightTheme of config.yml still apply after all files are merged, so on their background they override the theme's gui.theme; a theme can set its own gui.darkTheme and gui.lightTheme, which override those of config.yml. A theme file may only set gui.theme, gui.darkTheme and gui.lightTheme, and none of its values may be empty: a key without a value, or an empty list, would clear what config.yml sets, and a theme is only meant to set colors. Theme files are often downloaded or symlinked from elsewhere, so they are never rewritten. Themes written for older versions of lazygit have gui.authorColors, gui.branchColorPatterns or gui.branchColors directly in gui. Loading converts these in memory, by the same steps that migrate them in config.yml, and by no other migration, because the others reject YAML aliases, which themes may use to share colors. As in config.yml, a theme that has author colors or branch colors both there and in gui.theme can't be loaded. A theme that can't be loaded, because it breaks these rules or can't even be read, must not keep lazygit from starting, so the config is then loaded without it, and the error is kept for the GUI to report. The edit config action doesn't offer the theme file, so it keeps opening config.yml directly. The name in selected_theme.yml is looked up among the theme files ignoring case, and their spelling is used from then on. Otherwise a name edited by hand in the wrong case would load the theme on a file system that ignores case without showing it as selected, and wouldn't load it at all on other file systems. The applied theme is stored whenever the config loads successfully, instead of being derived from which files exist. A reload on focus that fails keeps the config it had, but it has already marked the files it got to as existing, so a theme file that reappears broken would count as applied although its colors aren't. AppConfigurer gains the getters that the GUI needs to show and report the theme, although the GUI doesn't use them yet.
The test driver started collecting toasts only once lazygit had gone idle for the first time. Toasts shown before that, for example while the first repo was being loaded, were shown in the status bar or dropped, so no test could check them. Start collecting them in NewGui instead, before anything can show a toast. A selected theme that can't be loaded is about to be reported with a toast at startup, and its test needs to see that toast.
Without this, a broken theme file silently leaves lazygit in the colors of config.yml, and nothing tells the user why their theme isn't there. Show an error toast naming the theme at startup and after every repo switch, which is when the theme is loaded. A toast only has room for a short line, so the full error, which names the file and what is wrong with it, goes to the log. It is a toast and not an alert because only one popup can be open at a time, so at startup an alert would compete with the intro or release notes popup. When selected_theme.yml itself can't be read, the name of the theme is unknown, so the toast just says that the selected theme couldn't be loaded.
Selecting a theme loads the config files with the new theme layer, but takes only the theme settings from the result. Those are gui.theme and the darkTheme and lightTheme overrides next to it, each taken as a whole so that nothing a previous theme set in them stays behind. Adopting the whole reloaded config would throw away settings that were changed at runtime, such as the branch sort order picked from its menu, and it would apply pending edits to config.yml without the work that the focus handler does for them (keybindings, side panels, the warning about settings that need a restart). Those edits are still picked up on the next focus, because the candidate is loaded from copies of the config files and so leaves the record of what was last loaded from them alone. Only a name that ListThemes returns is accepted, so that a name that differs only in case isn't remembered just because a case-insensitive file system finds the listed theme's file for it. A theme file that is deleted between the listing and the load is skipped by the load, so that case is reported as not found too, instead of remembering a theme that isn't applied. The choice is saved only after the theme has loaded, so that a broken theme file isn't selected again at the next start. A choice that can't be saved fails the selection and applies nothing, even when the reason is a lack of permission, which SaveAppState ignores: selected_theme.yml is read again on every repo switch, so a choice that wasn't saved would silently be undone there. A successful selection clears the error from the last load, because the theme it was about has either been replaced or has now loaded. Selecting the selected theme again is another attempt to load it, so when that fails, its error replaces the older one, which may describe a version of the file that has changed since. Such an error can now be about a theme file that is watched, so a reload on focus that loads the selected theme clears it too.
Selecting a theme while lazygit is running needs the same steps as a change of the terminal's background: apply the theme, and render again the views whose content was styled with the previous colors.
When the terminal's background changes, the views need their colors from the theme again, but nothing else that configureViewProperties sets: the frames, titles, tabs and jump labels come from other settings, which don't change with the background. Setting the colors after the loop that sets the frame runes and the background color, rather than in that loop, doesn't change the result: neither assignment reads what the other writes, and nothing is drawn until configureViewProperties returns. The separate line that set the commit description's foreground color is dropped rather than moved. The commit description view is in orderedViewNameMappings, so applyViewColors has already set that field to the same value, and nothing between the two writes reads or changes it.
Re-applying the theme called configureViewProperties, which also resets every view's title to its static one. The commit files view shows in its title the commit whose files it lists, so whenever the terminal switched between a dark and a light background while the commit files were open but not focused, their title lost the commit until the commits were next refreshed. A change of the theme only changes colors, so setting the views' colors again is all that it needs. Selecting a theme while lazygit is running is going to use this method too, and would otherwise lose the title in the same way.
…d changes Only set the views' colors again when the terminal's background changes Re-applying the theme called configureViewProperties, which also resets every view's title to its static one. The commit files view shows in its title the commit whose files it lists, so whenever the terminal switched between a dark and a light background while the commit files were open, their title lost the commit until the commits were next refreshed. A change of the theme only changes colors, so setting the views' colors again is all that it needs. Selecting a theme while lazygit is running is going to use this method too, and would otherwise lose the title in the same way.
So far a theme could only be selected by writing its name into selected_theme.yml and restarting lazygit. The theme menu lists the theme files, applies the chosen one right away and remembers it. The default key is '#': it is free in every context on every platform, terminals deliver it like any other printable key, and hex colors are what theme files are made of. The menu is also listed in the Global section of the keybindings menu, for keyboard layouts on which '#' is hard to type. After a switch, the theme is applied the same way as after a change of the terminal's background, which renders the list views, the status view and the main view of a focused side panel again, because their content is styled with theme colors when it is rendered. Frames and the selection highlight pick up the new colors on the next redraw anyway. The contexts of the staging, patch building and merge conflicts views render them when they are focused, so the current context is activated again. Choosing a menu item closes the menu first, which focuses that context again before the switch, and the renders this starts finish in the background with the old colors; activating it after the switch renders them once more, with the new colors. This also renders its search status again, which brings back the searching color while a search or filter is active there, because applying the theme resets the frame color of the focused view. The item of the selected theme shows the error of the last attempt to load it as its tooltip, which is the only place to see it without --debug. A selected theme that is listed but not in effect, because its file couldn't be loaded or only appeared later, is marked as not loaded, and choosing it loads it again. If that fails, the tooltip shows the new error from then on. One whose file is gone is marked as not found and can't be chosen, so that the menu doesn't pretend that no theme is selected.
Theme files are strict about their keys and values, sit between the global and the per-repo config files, and can't change the terminal background or the colors of a diff renderer. Colors for dark and light backgrounds are applied after all of these layers, which makes them win over the theme colors of any layer, and themes for older versions of lazygit are converted in memory, which changes what their error messages refer to. None of that can be discovered from the theme menu, and the catppuccin and rose-pine theme collections still tell users to combine config files with --use-config-file, so explain where theme files go, what they may contain and how they combine with the config, and point to this from the section about --use-config-file. A selected theme that can't be loaded at startup or on a repo switch is only reported with a short toast and a mark in the theme menu, so also explain what these mean and how to get the theme back once its file is fixed, which isn't the same in every case.
A theme for older versions of lazygit can leave gui.theme without a value, as the catppuccin themes-mergable files do, and have its authorColors directly in gui. Converting it failed with "yaml node in path is not a dictionary", because MoveYamlKey can't move a key into a null. Use an empty map for the null while converting; both set nothing when the theme is loaded, and the theme file is never rewritten.
Lazygit can already tell a diff renderer whether the terminal is dark or
light, through {{colorScheme}} in its command, and that follows the
terminal's background the same way gui.darkTheme and gui.lightTheme do.
Recommend it first, so that a renderer needs no second entry, and keep
cycling between renderers for the case that it can't cover: switching
the lazygit theme without changing the terminal's background.
The comment said that yaml merges only the value of the last merge key, which suggests that a mapping can have several. yaml rejects a second key with the same name, merge key or not, while decoding the node, and the node has been decoded by the time this runs.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description
Fixes #4561
This adds a way to switch between color themes while lazygit is running. Themes are YAML files in a
themesfolder in the config directory. Pressing#(also listed under Global in the?menu as "Select theme...") opens a menu of them; picking one recolors lazygit immediately and remembers the choice across restarts. No config file has to be edited to switch themes.I'm opening this as a draft: it's a working prototype for #4561, and I don't expect it to be merged (see CONTRIBUTING.md). It may also be useful to people who want the feature in their own fork. It is based on current master and builds on the recent theme work (#6062, #6063, #6065): theme files use the same
gui.theme/gui.darkTheme/gui.lightThemesettings asconfig.yml.How it works
<config dir>/themes/(lazygit --print-config-dirprints the config dir), e.g.mocha.ymlandlatte.yml. A theme's name is its file name without.yml.#. The menu lists(none)and every theme, with a radio button on the selected one. Choosing one applies it right away and shows the toastTheme: <name>.selected_theme.ymlnext tostate.yml, so lazygit starts with it the next time.A theme file is a partial lazygit config that may only contain theme settings:
Design decisions
LG_CONFIG_FILE), then the selected theme, then the per-repo config files, so per-repo colors keep working. As everywhere else,gui.darkTheme/gui.lightThemeare applied after all layers; a theme file can ship its own, which win over those inconfig.yml.gui.theme,gui.darkThemeandgui.lightTheme. Anything else is an error and the theme isn't applied: other keys (includinggui.colorScheme), misspelled theme keys, and empty or null values (which would silently wipe the user's own values). YAML anchors, aliases and merge keys work.authorColors,branchColorPatternsorbranchColorsdirectly undergui, as catppuccin'sthemes-mergablefiles have) are converted in memory with the same code that migratesconfig.yml.config.yml. lazygit doesn't write the user's config file, so the menu saves the choice in a state file of its own. It isn't stored instate.yml, because every running lazygit instance rewrites that file as a whole, which would undo a choice made in another instance.config.yml.(not loaded)with the reason as its tooltip, and choosing it again shows the full error. A theme whose file has disappeared shows as(not found).#: it's free in every context on every platform, arrives unmodified from all terminals and keyboard layouts I checked (including AltGr), and hints at hex colors. It has to be quoted in YAML (selectTheme: '#').Relation to #4561
Fixes and refactors along the way
Switching themes at runtime makes several existing problems routine, so they come first, as separate commits. Where possible, a commit demonstrating the bug (EXPECTED/ACTUAL) precedes its fix.
customCommandskey (per-repo config files, furtherLG_CONFIG_FILEentries).<<: *palette) inbranchColorPatternswere read as a pattern named<<, and the merged patterns were lost.Changes users without theme files may notice: toasts show ahead of waiting statuses; custom commands are no longer duplicated; merge keys in
branchColorPatternswork; the list of migration changes printed at startup may be in a slightly different order.Testing
pkg/integration/tests/theme/: the menu and the keybindings menu, the theme at startup, broken and missing themes, per-repo overrides, dark/light overrides in a theme file, reload on focus, runtime settings and view titles kept across a switch, and recoloring of the commits, staging and merge conflicts views.How to review
The commits are meant to be read one at a time: bug fixes first, then behavior-preserving refactors (a helper for files next to
state.yml, extracting the migration of theme keys, extractingreapplyThemeandapplyViewColors), then the theme layer, the load-error toast,AppConfig.SelectTheme, the menu, and the docs (new "Themes" section indocs-master/Config.md).Known limitations
{{colorScheme}}for diff renderers)..ymlfiles directly in thethemesfolder are listed (no subfolders).How this was made
I built this with an AI coding agent (Claude Code). I made the design decisions listed above, and every commit went through a separate review before the next one was started. I'm happy to answer questions about any part of it.
Please check if the PR fulfills these requirements
go generate ./...)keybinding.universal.selectThemekey, which reloads like the other keybindings)