Skip to content

fix: allow CLI config theme setting - #545

Open
YoneRai12 wants to merge 1 commit into
mainfrom
codex/fix-config-setter-for-new-theme-key
Open

fix: allow CLI config theme setting#545
YoneRai12 wants to merge 1 commit into
mainfrom
codex/fix-config-setter-for-new-theme-key

Conversation

@YoneRai12

Copy link
Copy Markdown
Owner

Motivation

  • テーマ設定が DEFAULT_CONFIGnormalize_config_key() には追加されているが、汎用設定セッターの parse_config_value()theme の処理が無く、set_cli_config_value("theme", ...)config set theme ... が失敗していたため一貫性を回復する。

Description

  • clients/cli/yonerai_cli/config.pyparse_config_value()theme ブランチを追加し、入力値を小文字化して THEMES = ("auto","dark","light","mono") に照合するようにした(不正値は ConfigError を送出)。
  • clients/cli/yonerai_cli/commands/config.pyCONFIG_KEY_CHOICES"theme" を追加し、config set theme ... が argparse の choices で拒否されないようにした。
  • 設定レポート生成 (build_config_report) と pretty 表示 (format_config_pretty) に theme を含めて、config show/set --json の出力で一貫して確認できるようにした。
  • 回帰テストを追加・更新して、set_cli_config_value("theme", ...) の永続化、無効値拒否、並びに CLI 選択肢の包含を検証するテストを tests/test_cli_theme.py に追加した。

Testing

  • PR 作業中に実行した自動化検証は全て成功している。
  • 単体テスト: PYTHONPATH=clients/cli pytest -q tests/test_cli_theme.py19 passed(成功)。
  • CLI 動作確認: PYTHONPATH=clients/cli python -m yonerai_cli config set theme dark --config-path /tmp/yonerai-theme-check-fixed.json --json は正常終了して、保存された JSON に "theme": "dark" が含まれることを確認した。
  • 静的/フォーマット/ビルド検証: python -m compileallruff チェック、git diff --check はパスした。
  • 追加差分に対するスキャン(追加行の秘密/ローカルパス検査、mojibake 検査、隠し Unicode 制御文字検査)を実施し問題なし。

追加情報

  • PR タイトル: fix: allow CLI config theme setting
  • コミット (ワークブランチ HEAD): b140285
  • ランタイム境界: 変更は CLI ローカル設定のパーサ/表示/テストに限定され、シークレットやプロダクション信頼ストア、live route、プロバイダ鍵等には影響を与えていない。
  • src/cogs/ora.pyreference_clawdbot は未変更。

Codex Task

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new "theme" configuration option to the CLI, allowing users to configure, parse, and display their preferred theme (auto, dark, light, or mono). It also adds corresponding unit tests to verify configuration persistence and validation. The review feedback suggests enhancing the theme parser to use theme_from_input so that theme aliases (such as Japanese translations or numeric representations) are supported consistently across the CLI, and adding tests to verify this alias resolution.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +209 to +213
if key == "theme":
normalized = raw.lower()
if normalized not in THEMES:
raise ConfigError("theme must be auto, dark, light, or mono.")
return normalized

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The current implementation of parse_config_value for "theme" only converts the input to lowercase and checks if it is in THEMES. However, other parts of the CLI (such as the /theme slash command and onboarding) support theme aliases like "々" (e.g., "々" or "々"), Japanese translations like "々", or numbers like "2". To maintain consistency across the CLI and prevent ConfigError when users set these valid aliases via config set theme, we should use theme_from_input to normalize the value.

Suggested change
if key == "theme":
normalized = raw.lower()
if normalized not in THEMES:
raise ConfigError("theme must be auto, dark, light, or mono.")
return normalized
if key == "theme":
from yonerai_cli.tui.themes import theme_from_input
normalized = theme_from_input(raw)
if normalized is None:
raise ConfigError("theme must be auto, dark, light, or mono.")
return normalized

Comment thread tests/test_cli_theme.py
Comment on lines +73 to +76
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"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Since we are supporting theme aliases (like "々" or "2") in the config parser, we should add test assertions to verify that these aliases are correctly resolved and persisted.

Suggested change
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"
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"
# Verify alias resolution
updated_alias = set_cli_config_value("theme", "々", config_path)
assert updated_alias["theme"] == "dark"

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e4a73b02f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +210 to +212
normalized = raw.lower()
if normalized not in THEMES:
raise ConfigError("theme must be auto, dark, light, or mono.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reuse the theme alias parser for config values

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 ダーク or yonerai config set theme 2; the first-launch picker and /テーマ path route through theme_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 raising ConfigError.

Useful? React with 👍 / 👎.

@YoneRai12

Copy link
Copy Markdown
Owner Author

Review intake classification on 2026-06-28:\n\n- valid-now, P2/UX: config set theme should normalize through heme_from_input so Japanese/numeric aliases match onboarding and /テーマ.\n- duplicate disposition: #544 is closed as duplicate; #545 is the canonical tracking PR for this theme-config UX fix.\n- blocker status: not P0/P1/security, not release/sync blocking.\n\nRecommended next action for this PR: refresh from current main, implement the shared alias parser path, keep the PR lane-scoped to config.py, commands/config.py, and ests/test_cli_theme.py, then rerun theme tests + ruff/compileall.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant