zach/z-bar-qt
1
1
Fork 0
Code Issues 23 Pull Requests 2 Actions Packages Projects 1 Releases Wiki Activity

95: Fix light/dark mode mismatch causes preset to not load #124

Merged
zach merged 2 commits from 95-fix-light-mode-scheme-mismatch into main 2026-06-14 21:27:27 +02:00
Conversation 3 Commits 2 Files Changed 1
+23 -1
1 changed files with 23 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cli/src/zshell/subcommands/scheme.py
+23 -1
View File
@@ -590,14 +590,36 @@ def generate(
raise typer.BadParameter(
f"Accent '{accent}' not available for '{p_scheme}:{p_variant}'. Available accents: {available}"
)
requested_mode = mode or config_mode
resolved_mode = requested_mode
if p_scheme in schemes:
meta = schemes[p_scheme]
variant = next(
(vari for vari in meta.variants if vari.id == p_variant), None
)
if variant and requested_mode not in variant.modes and variant.modes:
resolved_mode = sorted(variant.modes)[0]
zach marked this conversation as resolved
zach commented 2026-06-14 13:02:03 +02:00
Review
Copy Link
Copy Source

I personally think we shouldn't default to something here. Raising an exception would be better if the requested scheme is not found, maybe write exception trace to log file too. This is because the condition on line 601 should never return true and if it does then it means we are calling it incorrectly from QML/CLI.

I personally think we shouldn't default to something here. Raising an exception would be better if the requested scheme is not found, maybe write exception trace to log file too. This is because the condition on line 601 should never return true and if it does then it means we are calling it incorrectly from QML/CLI.
AramJonghu commented 2026-06-14 17:33:44 +02:00
Review
Copy Link
Copy Source

The reason it is there is because the current implementation usually only has a single mode per colorscheme preset. rosepine:dawn only has light mode for example. The current implementation allows less friction when selecting a preset that is not aligning to the mode you are at currently.

resolved_mode can only be light or dark (I am pretty sure).

We can think of a different solution, but that would be a more complex solution that'd be more dynamic (a possible consequence of the implementation of #125). Lets say user is on rosepine:main, which is a dark preset. If user switches to light, it could dynamically understand to move to rosepine:dawn. The current implementation does not do this.

I agree with error/exception trace to log file, good idea.

The reason it is there is because the current implementation usually only has a single mode per colorscheme preset. `rosepine:dawn` only has light mode for example. The current implementation allows less friction when selecting a preset that is not aligning to the mode you are at currently. `resolved_mode` can only be light or dark (I am pretty sure). We can think of a different solution, but that would be a more complex solution that'd be more dynamic (a possible consequence of the implementation of #125). Lets say user is on `rosepine:main`, which is a dark preset. If user switches to light, it could dynamically understand to move to `rosepine:dawn`. The current implementation does not do this. I agree with error/exception trace to log file, good idea.
palette_obj = get_palette(
p_scheme, p_variant, mode or config_mode, accent=accent
p_scheme, p_variant, resolved_mode, accent=accent
)
colors = palette_obj.colors
effective_mode = palette_obj.mode
name = palette_obj.scheme
flavor = palette_obj.variant
display_name = schemes[p_scheme].name
config["colors"]["presets"] = {
"name": display_name,
"variant": p_variant,
"accent": accent or "",
}
tmp = CONFIG.with_suffix(".json.tmp")
with tmp.open("w") as f:
json.dump(config, f, indent=4)
os.replace(tmp, CONFIG)
seed = hex_to_hct(colors.get("primary", "#000000").lstrip("#"))
else:
image_path = image_path or Path(WALL_PATH)