-
Notifications
You must be signed in to change notification settings - Fork 1
fix: allow CLI config theme setting #545
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -206,6 +206,11 @@ def parse_config_value(key: str, value: str) -> object: | |||||||||||||||||||||||
| if raw not in LANGUAGES: | ||||||||||||||||||||||||
| raise ConfigError("language must be ja or en.") | ||||||||||||||||||||||||
| return raw | ||||||||||||||||||||||||
| if key == "theme": | ||||||||||||||||||||||||
| normalized = raw.lower() | ||||||||||||||||||||||||
| if normalized not in THEMES: | ||||||||||||||||||||||||
| raise ConfigError("theme must be auto, dark, light, or mono.") | ||||||||||||||||||||||||
| return normalized | ||||||||||||||||||||||||
|
Comment on lines
+209
to
+213
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation of
Suggested change
|
||||||||||||||||||||||||
| if key == "provider_preference": | ||||||||||||||||||||||||
| if raw not in PROVIDER_PREFERENCES: | ||||||||||||||||||||||||
| raise ConfigError("provider must be auto, mock, local, openai-compatible, anthropic, or gemini.") | ||||||||||||||||||||||||
|
|
@@ -312,6 +317,7 @@ def build_config_report(config: Mapping[str, object], *, exists: bool) -> dict[s | |||||||||||||||||||||||
| "secrets_supported": False, | ||||||||||||||||||||||||
| "config": { | ||||||||||||||||||||||||
| "language": validated["language"], | ||||||||||||||||||||||||
| "theme": validated["theme"], | ||||||||||||||||||||||||
| "command_display_mode": validated["command_display_mode"], | ||||||||||||||||||||||||
| "provider_preference": validated["provider_preference"], | ||||||||||||||||||||||||
| "model_preference": validated["model_preference"], | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import io | ||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||||||
|
|
@@ -21,7 +22,15 @@ | |||||||||||||||||||||||||
| if str(path) not in sys.path: | ||||||||||||||||||||||||||
| sys.path.insert(0, str(path)) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from yonerai_cli.config import DEFAULT_CONFIG, ConfigError, THEMES, save_cli_config, validate_cli_config | ||||||||||||||||||||||||||
| from yonerai_cli.config import ( | ||||||||||||||||||||||||||
| DEFAULT_CONFIG, | ||||||||||||||||||||||||||
| THEMES, | ||||||||||||||||||||||||||
| ConfigError, | ||||||||||||||||||||||||||
| save_cli_config, | ||||||||||||||||||||||||||
| set_cli_config_value, | ||||||||||||||||||||||||||
| validate_cli_config, | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| from yonerai_cli.commands.config import CONFIG_KEY_CHOICES | ||||||||||||||||||||||||||
| from yonerai_cli.startup_home import render_startup_home_header | ||||||||||||||||||||||||||
| from yonerai_cli.tui.themes import normalize_theme, theme_from_input, theme_palette, theme_uses_truecolor | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -58,6 +67,26 @@ def test_invalid_theme_rejected() -> None: | |||||||||||||||||||||||||
| validate_cli_config(cfg) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def test_config_setter_persists_theme(tmp_path: Path) -> None: | ||||||||||||||||||||||||||
| config_path = tmp_path / "config.json" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| updated = set_cli_config_value("theme", "DARK", config_path) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| assert updated["theme"] == "dark" | ||||||||||||||||||||||||||
| assert json.loads(config_path.read_text(encoding="utf-8"))["theme"] == "dark" | ||||||||||||||||||||||||||
|
Comment on lines
+73
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are supporting theme aliases (like
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def test_config_setter_rejects_invalid_theme(tmp_path: Path) -> None: | ||||||||||||||||||||||||||
| config_path = tmp_path / "config.json" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| with pytest.raises(ConfigError, match="theme must be auto, dark, light, or mono"): | ||||||||||||||||||||||||||
| set_cli_config_value("theme", "neon", config_path) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def test_config_command_choices_include_theme() -> None: | ||||||||||||||||||||||||||
| assert "theme" in CONFIG_KEY_CHOICES | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # --- palette / rendering --- | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When users follow the Japanese-first theme UI, this new config path still rejects values that the existing theme flows accept, such as
yonerai config set theme ダークoryonerai config set theme 2; the first-launch picker and/テーマpath route throughtheme_from_input, but this branch only lowercases and checks canonical English names. That leaves the non-interactive theme setter unusable for the same values already exposed by the theme UI, so the value should be normalized through the shared theme alias parser before raisingConfigError.Useful? React with 👍 / 👎.