fix: Memory Display Error Issue#184
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Nov 27, 2025
Merged
Conversation
Reviewer's GuideFixes locale-dependent number formatting and memory display by correctly handling system decimal/grouping separators, avoiding blind removal of commas, and reformatting memory entries from underlying numeric values instead of already-localized text. Sequence diagram for localized memory evaluation and displaysequenceDiagram
actor User
participant InputEdit
participant Settings
participant Evaluator
participant MemoryStore
participant MemoryWidget
participant Utils
participant DMath
User->>InputEdit: type 11.111,04
User->>InputEdit: press_memory_recall_or_store
InputEdit->>Settings: getSystemDecimalSymbol()
Settings-->>InputEdit: decSym
InputEdit->>Settings: getSystemDigitGroupingSymbol()
Settings-->>InputEdit: grpSym
Note over InputEdit: Replace locale-specific operators, then use decSym/grpSym to normalize expression
InputEdit->>InputEdit: replace(decSym, placeholder)
InputEdit->>InputEdit: replace(grpSym, "")
InputEdit->>InputEdit: replace(placeholder, ".")
InputEdit->>Evaluator: setExpression(normalized_expression)
InputEdit->>Evaluator: evalUpdateAns()
Evaluator-->>InputEdit: Quantity result
InputEdit->>MemoryStore: save Quantity result
User->>MemoryWidget: open_memory_view_or_change_separators
MemoryWidget->>MemoryStore: getList()
MemoryStore-->>MemoryWidget: list_of_Quantity
alt programmer_mode
MemoryWidget->>Utils: formatThousandsSeparatorsPro(programmerResult, base)
Utils-->>MemoryWidget: formatted_string
else normal_mode
MemoryWidget->>DMath: format(Quantity, General_Precision)
DMath-->>MemoryWidget: result_string
MemoryWidget->>Utils: formatThousandsSeparators(result_string)
Utils-->>MemoryWidget: formatted_string
end
MemoryWidget->>MemoryWidget: setitemwordwrap(formatted_string, index)
MemoryWidget->>MemoryWidget: setTextLabel(formatted_string)
MemoryWidget-->>User: correctly_localized_memory_display
Updated class diagram for locale-aware formatting and memory displayclassDiagram
class Settings {
+static Settings instance()
+QString getSystemDecimalSymbol()
+QString getSystemDigitGroupingSymbol()
+bool getSystemDigitGrouping()
+int programmerBase
}
class DSettingsAlt {
+static DSettingsAlt instance()
+int getSeparate()
}
class Utils {
+static QString formatThousandsSeparators(QString str)
+static QString formatThousandsSeparatorsPro(QString str, int base)
}
class InputEdit {
-Evaluator m_evaluator
+QPair~bool, Quantity~ getMemoryAnswer()
}
class MemoryWidget {
-int m_calculatormode
-QListWidget m_listwidget
-MemoryPublic m_memorypublic
-int m_precision
+void resetLabelBySeparator()
}
class DMath {
+static QString format(Quantity value, Quantity_Format format)
}
class Quantity {
<<value_object>>
+Format Format
}
Settings <.. InputEdit : uses
Settings <.. Utils : uses
DSettingsAlt <.. Utils : uses
Utils <.. MemoryWidget : uses
DMath <.. MemoryWidget : uses
Quantity <.. InputEdit : returns
Quantity <.. MemoryWidget : uses
MemoryWidget <.. MemoryPublic : has
InputEdit <.. Evaluator : has
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
InputEdit::getMemoryAnswer, consider avoiding the magic placeholder characterQChar(0x1D)by defining a named constant (or using a less obscure sentinel) and adding a brief note about why it's guaranteed not to appear in user input, to reduce the risk of accidental collisions or future regressions. - In
MemoryWidget::resetLabelBySeparator, you repeatedly callSettings::instance()and recompute formatting settings inside the loop; caching the relevant settings (and possibly the base) before the loop would simplify the code and avoid redundant lookups.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `InputEdit::getMemoryAnswer`, consider avoiding the magic placeholder character `QChar(0x1D)` by defining a named constant (or using a less obscure sentinel) and adding a brief note about why it's guaranteed not to appear in user input, to reduce the risk of accidental collisions or future regressions.
- In `MemoryWidget::resetLabelBySeparator`, you repeatedly call `Settings::instance()` and recompute formatting settings inside the loop; caching the relevant settings (and possibly the base) before the loop would simplify the code and avoid redundant lookups.
## Individual Comments
### Comment 1
<location> `src/utils.cpp:105-115` </location>
<code_context>
+ fractional = input.mid(decIdx + 1);
+ integerPart = input.left(decIdx);
+ }
+ // 仅清理整数部分中可能已存在的分组符号（'.'、','、全角'，'、空格、系统分组符），小数部分完全保持原样
+ const QChar fullWidthComma(0xFF0C);
+ if (!grpSym.isEmpty())
+ integerPart.remove(grpSym);
+ integerPart.remove(QLatin1Char('.'));
+ integerPart.remove(QLatin1Char(','));
+ integerPart.remove(fullWidthComma);
+ integerPart.remove(QLatin1Char(' '));
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Consider broadening grouping cleanup to handle other common spacing/grouping characters.
Currently this only removes '.', ',', full-width comma, plain space, and the system grouping symbol. In some locales, pasted numbers may include non‑breaking space (U+00A0), thin space (U+2009), narrow no‑break space, etc. as group separators.
Consider either:
- Normalizing all Unicode spaces in the integer part to plain spaces, then removing them, or
- Explicitly removing a small set of additional common grouping characters (NBSP, thin space, narrow no‑break space).
This will make parsing more robust for copy/paste and mixed‑locale inputs.
```suggestion
}
+ // 仅清理整数部分中可能已存在的分组符号（'.'、','、全角'，'、各种空格、系统分组符），小数部分完全保持原样
+ const QChar fullWidthComma(0xFF0C);
+ const QChar nonBreakingSpace(0x00A0); // NBSP
+ const QChar thinSpace(0x2009); // THIN SPACE
+ const QChar narrowNoBreakSpace(0x202F); // NARROW NO-BREAK SPACE
+
+ if (!grpSym.isEmpty())
+ integerPart.remove(grpSym);
+ integerPart.remove(QLatin1Char('.'));
+ integerPart.remove(QLatin1Char(','));
+ integerPart.remove(fullWidthComma);
+ integerPart.remove(QLatin1Char(' '));
+ integerPart.remove(nonBreakingSpace);
+ integerPart.remove(thinSpace);
+ integerPart.remove(narrowNoBreakSpace);
+ }
+
```
</issue_to_address>
### Comment 2
<location> `src/widgets/inputedit.cpp:1138` </location>
<code_context>
+ const auto sys = Settings::instance();
+ const QString decSym = sys->getSystemDecimalSymbol();
+ const QString grpSym = sys->getSystemDigitGroupingSymbol();
+ const QString decimalPlaceholder = QString(QChar(0x1D));
+
+ if (!decSym.isEmpty() && decSym != QLatin1String("."))
</code_context>
<issue_to_address>
**suggestion:** Using U+001D as a decimal placeholder risks clashing with existing content.
Using a control character as the placeholder could still collide with rare but valid input (e.g. pasted content that already contains U+001D), causing subtle data corruption.
To minimize this risk, prefer either:
- A private-use code point (e.g. U+E000), or
- A longer, clearly synthetic sentinel string (e.g. "__DEC_PLACEHOLDER__").
Both options greatly reduce the chance of overlapping with real user input.
```suggestion
// Use a synthetic sentinel string instead of a control character to minimize
// the risk of collisions with pasted or user-entered content.
const QString decimalPlaceholder = QStringLiteral("__DEC_PLACEHOLDER__");
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
JWWTSL
force-pushed
the
master
branch
2 times, most recently
from
f59e11a to
29e06b9
Compare
log: When the decimal separator is set to a comma and the grouping separator is set to a period, `11.111,04` is displayed in memory as `11,11104`. bug: https://pms.uniontech.com/bug-view-342051.html
deepin pr auto review我来对这段代码的修改进行审查，主要从语法逻辑、代码质量、性能和安全几个方面进行分析：
具体改进建议：
static constexpr const char* DECIMAL_PLACEHOLDER = "__DEC_PLACEHOLDER__";
QString cleanSeparators(const QString& input, const QString& decSym, const QString& grpSym) {
QString result = input;
const QChar fullWidthComma(0xFF0C);
if (!grpSym.isEmpty())
result.remove(grpSym);
result.remove(QLatin1Char('.'));
result.remove(QLatin1Char(','));
result.remove(fullWidthComma);
result.remove(QLatin1Char(' '));
return result;
}
using namespace QtLiterals;
expression = symbolComplement(expressionText())
% QString::fromUtf8("＋").replace("+")
% QString::fromUtf8("－").replace("-")
% QString::fromUtf8("×").replace("*")
% QString::fromUtf8("÷").replace("/");
if (decSym == grpSym) {
qWarning() << "Decimal symbol and grouping symbol cannot be the same";
return {};
}总体来说，这次修改显著提升了代码的健壮性和本地化支持，主要改进点合理且必要。代码质量良好，但仍有小幅优化空间。 |
lzwind
approved these changes
Nov 27, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JWWTSL, lzwind 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 |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
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
3 participants
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.
log: When the decimal separator is set to a comma and the grouping separator is set to a period,
11.111,04is displayed in memory as11,11104.Summary by Sourcery
Fix incorrect handling of locale-specific decimal and grouping separators when formatting and displaying memory values.
Bug Fixes: