fix: use dynamic integer range from fcitx5 config properties#143
fix: use dynamic integer range from fcitx5 config properties#143deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuidePropagates fcitx5 Integer option IntMin/IntMax properties through the config proxy into the QML SpinBox, and updates the SpinBox to be editable, slightly wider, and to respect a dynamic integer range with sensible fallbacks. Sequence diagram for Integer option range propagation from fcitx5 to QML SpinBoxsequenceDiagram
actor User
participant DetailConfigItem_qml
participant Fcitx5ConfigProxy
participant Fcitx5
User->>DetailConfigItem_qml: Open Integer config item UI
DetailConfigItem_qml->>Fcitx5ConfigProxy: globalConfigOptions(type, allowInputMethodOverride)
Fcitx5ConfigProxy->>Fcitx5: Request config options
Fcitx5-->>Fcitx5ConfigProxy: Option data with properties including IntMin, IntMax
Fcitx5ConfigProxy->>Fcitx5ConfigProxy: Build item QVariantMap
Fcitx5ConfigProxy->>Fcitx5ConfigProxy: If type Integer, copy IntMin to intMin and IntMax to intMax
Fcitx5ConfigProxy-->>DetailConfigItem_qml: List of options including intMin, intMax
DetailConfigItem_qml->>DetailConfigItem_qml: Bind SpinBox.from to modelData.intMin or 0
DetailConfigItem_qml->>DetailConfigItem_qml: Bind SpinBox.to to modelData.intMax or 9999
User->>DetailConfigItem_qml: Edit SpinBox value (within [from, to])
DetailConfigItem_qml->>Fcitx5ConfigProxy: setValue(section, name, newValue)
Fcitx5ConfigProxy->>Fcitx5: Apply updated Integer config
Fcitx5-->>User: Integer option updated with enforced range
Class diagram for Fcitx5ConfigProxy and Integer config option item structureclassDiagram
class Fcitx5ConfigProxy {
+QVariantList globalConfigOptions(QString type, bool allowInputMethodOverride)
+void setValue(QString section, QString name, QVariant value)
}
class ConfigOptionItem {
+QString type
+QVariant value
+int intMin
+int intMax
}
Fcitx5ConfigProxy "1" --> "*" ConfigOptionItem : builds_items_with_intMin_intMax_for_Integer_type
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider avoiding the hardcoded default range (0–9999) in the SpinBox and instead deriving a safer fallback (e.g., based on the current value or a shared constant) to avoid silently clamping valid existing values when IntMin/IntMax are not provided.
- It may be worth validating that intMin is not greater than intMax when reading the Integer properties and handling any inconsistent configuration (e.g., by swapping or ignoring the bounds) to prevent unexpected SpinBox behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider avoiding the hardcoded default range (0–9999) in the SpinBox and instead deriving a safer fallback (e.g., based on the current value or a shared constant) to avoid silently clamping valid existing values when IntMin/IntMax are not provided.
- It may be worth validating that intMin is not greater than intMax when reading the Integer properties and handling any inconsistent configuration (e.g., by swapping or ignoring the bounds) to prevent unexpected SpinBox behavior.
## Individual Comments
### Comment 1
<location path="src/dcc-fcitx5configtool/qml/DetailConfigItem.qml" line_range="118-119" />
<code_context>
+ editable: true
+ width: 75
+ implicitWidth: 75
+ from: modelData.intMin !== undefined ? modelData.intMin : 0
+ to: modelData.intMax !== undefined ? modelData.intMax : 9999
value: parseInt(modelData.value)
onValueChanged: {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Hard-coded default range 0–9999 may not match actual valid range for all integer options.
If `IntMin`/`IntMax` are missing, defaulting to `[0, 9999]` can prevent valid negative or large values and silently clamp user input. Consider either not constraining the SpinBox when no range is defined, or using a more permissive default that aligns with the config semantics.
```suggestion
from: modelData.intMin !== undefined ? modelData.intMin : -2147483648
to: modelData.intMax !== undefined ? modelData.intMax : 2147483647
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| from: modelData.intMin !== undefined ? modelData.intMin : 0 | ||
| to: modelData.intMax !== undefined ? modelData.intMax : 9999 |
There was a problem hiding this comment.
suggestion (bug_risk): Hard-coded default range 0–9999 may not match actual valid range for all integer options.
If IntMin/IntMax are missing, defaulting to [0, 9999] can prevent valid negative or large values and silently clamp user input. Consider either not constraining the SpinBox when no range is defined, or using a more permissive default that aligns with the config semantics.
| from: modelData.intMin !== undefined ? modelData.intMin : 0 | |
| to: modelData.intMax !== undefined ? modelData.intMax : 9999 | |
| from: modelData.intMin !== undefined ? modelData.intMin : -2147483648 | |
| to: modelData.intMax !== undefined ? modelData.intMax : 2147483647 |
Pass IntMax/IntMin properties for Integer type options to QML SpinBox, enable editable input and set fixed width to prevent layout stretching. 将Integer类型选项的IntMax/IntMin属性传递到QML SpinBox,启用可编辑输入并设置固定宽度防止布局拉伸。 Log: SpinBox支持动态整数范围和可编辑输入 PMS: BUG-357563 Influence: Integer类型配置项的SpinBox现在使用fcitx5配置中定义的实际范围,支持手动输入数值。
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, wyu71 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
deepin pr auto review这段代码的diff主要做了两处修改:
下面是对代码的详细审查和改进建议: 1. 语法逻辑审查优点:
问题与改进: // 当前代码
if (option.type() == "Integer") {
bool ok = false;
int maxVal = properties.contains("IntMax") ? properties["IntMax"].toInt(&ok) : 9999;
item["intMax"] = ok ? maxVal : 9999;
int minVal = properties.contains("IntMin") ? properties["IntMin"].toInt(&ok) : 0;
item["intMin"] = ok ? minVal : 0;
}改进建议:
改进后的代码: if (option.type() == "Integer") {
bool ok = false;
const int DEFAULT_MIN = 0;
const int DEFAULT_MAX = 9999;
// 处理最大值
int maxVal = DEFAULT_MAX;
if (properties.contains("IntMax")) {
maxVal = properties["IntMax"].toInt(&ok);
if (!ok) maxVal = DEFAULT_MAX;
}
item["intMax"] = maxVal;
// 处理最小值
int minVal = DEFAULT_MIN;
if (properties.contains("IntMin")) {
ok = false; // 重置ok标志
minVal = properties["IntMin"].toInt(&ok);
if (!ok) minVal = DEFAULT_MIN;
}
item["intMin"] = minVal;
// 确保最小值不大于最大值
if (item["intMin"].toInt() > item["intMax"].toInt()) {
qWarning() << "Invalid integer range: min > max, swapping values";
int temp = item["intMin"].toInt();
item["intMin"] = item["intMax"];
item["intMax"] = temp;
}
}2. 代码质量审查QML部分: D.SpinBox {
editable: true
width: 75
implicitWidth: 75
from: modelData.intMin !== undefined ? modelData.intMin : 0
to: modelData.intMax !== undefined ? modelData.intMax : 9999
value: parseInt(modelData.value)改进建议:
改进后的QML代码: D.SpinBox {
id: spinBox
editable: true
width: 75
implicitWidth: 75
from: modelData.intMin !== undefined ? modelData.intMin : 0
to: modelData.intMax !== undefined ? modelData.intMax : 9999
value: parseInt(modelData.value) || 0
stepSize: 1
validator: IntValidator {
bottom: spinBox.from
top: spinBox.to
}
onValueChanged: {
if (value >= from && value <= to) {
dccData.fcitx5ConfigProxy.setValue(...)
}
}
}3. 代码性能审查
4. 代码安全审查
安全改进示例: if (option.type() == "Integer") {
bool ok = false;
const int DEFAULT_MIN = 0;
const int DEFAULT_MAX = 9999;
// 安全地获取最大值
int maxVal = DEFAULT_MAX;
if (properties.contains("IntMax")) {
bool maxOk = false;
int tempMax = properties["IntMax"].toInt(&maxOk);
if (maxOk && tempMax >= DEFAULT_MIN && tempMax <= std::numeric_limits<int>::max()) {
maxVal = tempMax;
}
}
item["intMax"] = maxVal;
// 安全地获取最小值
int minVal = DEFAULT_MIN;
if (properties.contains("IntMin")) {
bool minOk = false;
int tempMin = properties["IntMin"].toInt(&minOk);
if (minOk && tempMin >= std::numeric_limits<int>::min() && tempMin <= DEFAULT_MAX) {
minVal = tempMin;
}
}
item["intMin"] = minVal;
// 确保范围有效
if (minVal > maxVal) {
qWarning() << "Invalid integer range, using defaults";
item["intMin"] = DEFAULT_MIN;
item["intMax"] = DEFAULT_MAX;
}
}5. 其他建议
总结来说,这段代码的基本功能是正确的,但在错误处理、边界条件检查和代码健壮性方面还有改进空间。建议采纳上述改进建议以提高代码质量和安全性。 |
|
This pr force merged! (status: blocked) |
Pass IntMax/IntMin properties for Integer type options to QML SpinBox, enable editable input and set fixed width to prevent layout stretching.
将Integer类型选项的IntMax/IntMin属性传递到QML SpinBox,启用可编辑输入并设置固定宽度防止布局拉伸。
Log: SpinBox支持动态整数范围和可编辑输入
PMS: BUG-357563
Influence: Integer类型配置项的SpinBox现在使用fcitx5配置中定义的实际范围,支持手动输入数值。
Summary by Sourcery
Use fcitx5 Integer option metadata to define dynamic ranges for SpinBox-based integer configuration entries and allow direct value editing in the config UI.
New Features:
Bug Fixes:
Enhancements: