feat: 同时显示ConfigItem配置值和外显值的设置卡#2222
Open
ShadowLemoon wants to merge 2 commits into
Open
Conversation
Contributor
📝 Walkthrough演进概览此PR更新了类型导入并引入了一个新的可编辑组合框设置卡小部件，该小部件在右侧显示标签中显示所选选项的显示文本，同时支持自由文本输入并保持底层值与UI同步。 变更内容ValueDisplayEditableComboBoxSettingCard 小部件
代码审查工作量评估🎯 3 (中等) | ⏱️ ~20 分钟 兔子的诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
[skip ci]
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/one_dragon_qt/widgets/setting_card/value_display_editable_combo_box_setting_card.py (2)
36-48: ⚡ Quick win建议使用
super().__init__(...)替代显式类名调用直接调用
EditableComboBoxSettingCard.__init__(self, ...)绕过了 MRO，在未来引入多重继承时可能产生问题，且不符合 Python 惯用写法。♻️ 建议改写
- EditableComboBoxSettingCard.__init__( - self, - icon=icon, - title=title, - content=content, - icon_size=icon_size, - margins=margins, - options_enum=options_enum, - options_list=options_list, - input_placeholder=input_placeholder, - tooltip=tooltip, - parent=parent, - ) + super().__init__( + icon=icon, + title=title, + content=content, + icon_size=icon_size, + margins=margins, + options_enum=options_enum, + options_list=options_list, + input_placeholder=input_placeholder, + tooltip=tooltip, + parent=parent, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/one_dragon_qt/widgets/setting_card/value_display_editable_combo_box_setting_card.py` around lines 36 - 48, Replace the explicit base-class constructor call EditableComboBoxSettingCard.__init__(self, ...) with the standard cooperative call using super().__init__(...), i.e. invoke super().__init__(icon=icon, title=title, content=content, icon_size=icon_size, margins=margins, options_enum=options_enum, options_list=options_list, input_placeholder=input_placeholder, tooltip=tooltip, parent=parent) inside the class initializer so the __init__ participates correctly in Python's MRO and supports future multiple inheritance changes.
100-105: ⚡ Quick win
set_options_by_list中存在重复的hasattr检查两个连续的
if hasattr(self, 'display_label')条件完全相同，可合并为一个if块。♻️ 建议改写
def set_options_by_list(self, options: list[ConfigItem]) -> None: super().set_options_by_list(options) - if hasattr(self, 'display_label'): - self._update_combo_box_minimum_width() - if hasattr(self, 'display_label'): - self._sync_display_name(self.getValue()) + if hasattr(self, 'display_label'): + self._update_combo_box_minimum_width() + self._sync_display_name(self.getValue())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/one_dragon_qt/widgets/setting_card/value_display_editable_combo_box_setting_card.py` around lines 100 - 105, In set_options_by_list, remove the duplicated hasattr(self, 'display_label') checks and combine them into a single if block so both _update_combo_box_minimum_width() and _sync_display_name(self.getValue()) are executed when display_label exists; update the method to call _update_combo_box_minimum_width() then _sync_display_name(self.getValue()) inside one if and leave the call to super().set_options_by_list(options) untouched.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/one_dragon_qt/widgets/setting_card/value_display_editable_combo_box_setting_card.py`:
- Around line 126-127: getValue currently annotates its return type as object
but it always returns a str because it calls _normalize_value on
combo_box.text().strip(); update the signature of getValue (the method named
getValue) to return str instead of object so the annotation matches the actual
return type from _normalize_value and combo_box.text().strip().
- Line 31: The constructor parameter parent is missing a type annotation; update
the signature (in class ValueDisplayEditableComboBoxSettingCard) from
parent=None to parent: Optional[QWidget] = None (or parent: Optional[QObject] =
None if the class expects a QObject parent), and add the required imports (from
typing import Optional and the appropriate Qt class such as QWidget/QObject) at
the top of the file.
- Around line 23-24: Update the __init__ signature in class
ValueDisplayEditableComboBoxSettingCard to annotate parent as parent: QWidget |
None = None (and import QWidget from your GUI lib) instead of untyped
parent=None; change the return annotation of getValue() to -> str because
_normalize_value always returns a string; and simplify the repeated
hasattr(self, 'display_label') checks around the display_label usage (currently
in the block that initializes/updates the label) by merging them into a single
hasattr check to avoid duplicate conditionals.
- Around line 136-148: _sync_display_name is comparing item.value (which can be
non-string) to value (always a str), causing the match to fail and display_label
to remain empty; update the comparison in _sync_display_name to compare strings
by using str(item.value) == value (or handle None by converting to ''), so the
loop can find the correct item from self._opts_list and set
display_label/text/toolTip and call _update_combo_box_text_margins correctly;
check related places that produce value (like _apply_value, setValue,
_normalize_value) to ensure they still provide a str and keep this string
comparison consistent.
---
Nitpick comments:
In
`@src/one_dragon_qt/widgets/setting_card/value_display_editable_combo_box_setting_card.py`:
- Around line 36-48: Replace the explicit base-class constructor call
EditableComboBoxSettingCard.__init__(self, ...) with the standard cooperative
call using super().__init__(...), i.e. invoke super().__init__(icon=icon,
title=title, content=content, icon_size=icon_size, margins=margins,
options_enum=options_enum, options_list=options_list,
input_placeholder=input_placeholder, tooltip=tooltip, parent=parent) inside the
class initializer so the __init__ participates correctly in Python's MRO and
supports future multiple inheritance changes.
- Around line 100-105: In set_options_by_list, remove the duplicated
hasattr(self, 'display_label') checks and combine them into a single if block so
both _update_combo_box_minimum_width() and _sync_display_name(self.getValue())
are executed when display_label exists; update the method to call
_update_combo_box_minimum_width() then _sync_display_name(self.getValue())
inside one if and leave the call to super().set_options_by_list(options)
untouched.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ac16b018-5659-4efd-87a5-4ca2ed65cbdc
📒 Files selected for processing (2)
deploy/module_manifest.pysrc/one_dragon_qt/widgets/setting_card/value_display_editable_combo_box_setting_card.py
Comment on lines
+136
to
+148
| def _sync_display_name(self, value: object) -> None: | ||
| if not hasattr(self, 'display_label'): | ||
| return | ||
|
|
||
| display = '' | ||
| for item in self._opts_list: | ||
| if item.value == value: | ||
| display = item.ui_text | ||
| break | ||
| self.display_label.setText(display) | ||
| self.display_label.setToolTip(display) | ||
| self.display_label.setVisible(bool(display)) | ||
| self._update_combo_box_text_margins(display) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. 确认 ConfigItem.value 的类型定义
ast-grep --pattern 'class ConfigItem {
$$$
}'
# 2. 确认父类如何向 combo_box 存储 item data
rg -n "addItem\|setItemData\|itemData" --type py -C 3 src/one_dragon_qt/widgets/setting_card/
# 3. 查看 _opts_list 的构建方式
rg -n "_opts_list" --type py -C 3 src/one_dragon_qt/widgets/setting_card/Repository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 12240
🏁 Script executed:
#!/bin/bash
# Find ConfigItem class definition
rg -n "class ConfigItem" --type py -A 10Repository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 942
🏁 Script executed:
#!/bin/bash
# Check the target file's _apply_value, setValue, and _on_text_changed methods
cat -n src/one_dragon_qt/widgets/setting_card/value_display_editable_combo_box_setting_card.py | head -200Repository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 8223
🏁 Script executed:
#!/bin/bash
# Check the parent class EditableComboBoxSettingCard for _apply_value
rg -n "_apply_value\|def setValue" src/one_dragon_qt/widgets/setting_card/editable_combo_box_setting_card.py --type py -B 2 -A 8Repository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 70
_sync_display_name 的类型比较导致显示标签为空
方法接收的 value 参数始终为 str 类型（来自 _apply_value 的 str(value) 转换、setValue 的 str(value) 转换、以及 _normalize_value 的返回值），但第 142 行比较 item.value == value 时，item.value 为 Any 类型（可能是 int、Enum 等）。当 item.value 非字符串时，等式永远不成立，外显标签将始终为空。
改为 str(item.value) == value：
🛠️ 修复方案
def _sync_display_name(self, value: object) -> None:
if not hasattr(self, 'display_label'):
return
display = ''
for item in self._opts_list:
- if item.value == value:
+ if str(item.value) == value:
display = item.ui_text
break🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/one_dragon_qt/widgets/setting_card/value_display_editable_combo_box_setting_card.py`
around lines 136 - 148, _sync_display_name is comparing item.value (which can be
non-string) to value (always a str), causing the match to fail and display_label
to remain empty; update the comparison in _sync_display_name to compare strings
by using str(item.value) == value (or handle None by converting to ''), so the
loop can find the correct item from self._opts_list and set
display_label/text/toolTip and call _update_combo_box_text_margins correctly;
check related places that produce value (like _apply_value, setValue,
_normalize_value) to ensure they still provide a str and keep this string
comparison consistent.
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.
Summary by CodeRabbit
新增功能
其他改进