feat: Adapt to regional format synchronization #178
Merged
Conversation
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
|
TAG Bot TAG: 6.5.28 |
Reviewer's GuideThis PR adds system-wide regional format synchronization by fetching decimal and grouping symbols from the Control Center (via D-Bus with QLocale fallback) and updating all calculator components to use those symbols instead of hard-coded "." and "," separators. Sequence diagram for fetching regional format symbols from Control CentersequenceDiagram
participant Calculator
participant Settings
participant D_Bus_org_deepin_dde_Format1 as D-Bus_org.deepin.dde.Format1
participant QLocale
Calculator->>Settings: getSystemDecimalSymbol()
Settings->>D_Bus_org_deepin_dde_Format1: Request DecimalSymbol
alt D-Bus returns value
D_Bus_org_deepin_dde_Format1-->>Settings: DecimalSymbol
Settings-->>Calculator: DecimalSymbol
else D-Bus fails
Settings->>QLocale: Get decimalPoint()
QLocale-->>Settings: decimalPoint
Settings-->>Calculator: decimalPoint
end
Calculator->>Settings: getSystemDigitGroupingSymbol()
Settings->>D_Bus_org_deepin_dde_Format1: Request DigitGroupingSymbol
alt D-Bus returns value
D_Bus_org_deepin_dde_Format1-->>Settings: DigitGroupingSymbol
Settings-->>Calculator: DigitGroupingSymbol
else D-Bus fails
Settings->>QLocale: Get groupSeparator()
QLocale-->>Settings: groupSeparator
Settings-->>Calculator: groupSeparator
end
Class diagram for Settings and regional format symbol accessclassDiagram
class Settings {
+QString getSystemDecimalSymbol() const
+QString getSystemDigitGroupingSymbol() const
+bool getSystemDigitGrouping() const
}
Settings <.. Calculator : uses
Settings <.. InputEdit : uses
Settings <.. ExpressionBar : uses
Settings <.. SciExpressionBar : uses
Settings <.. ProgrammerKeypad : uses
Settings <.. ScientificKeyPad : uses
Settings <.. BasicKeypad : uses
Settings <.. Utils : uses
Class diagram for InputEdit normalization and filteringclassDiagram
class InputEdit {
+void handleTextChanged(const QString &text)
+QString pointFaultTolerance(const QString &text)
+QString formatExpression(const int &probase, const QString &text)
-void normalizeInput(const QString &text)
}
InputEdit <.. Settings : uses
Class diagram for ExpressionBar and SciExpressionBar format adaptationclassDiagram
class ExpressionBar {
+void enterPointEvent()
+QString formatExpression(const QString &text)
+QString pointFaultTolerance(const QString &text)
+void expressionCheck()
}
class SciExpressionBar {
+void enterPointEvent()
+QString formatExpression(const QString &text)
+QString pointFaultTolerance(const QString &text)
+void expressionCheck()
}
ExpressionBar <.. Settings : uses
SciExpressionBar <.. Settings : uses
Class diagram for keypad classes updating decimal symbolclassDiagram
class ProgrammerKeypad {
+void initButtons()
+void radixChanged(int row)
}
class ScientificKeyPad {
+void initButtons()
}
class BasicKeypad {
+void initButtons()
}
ProgrammerKeypad <.. Settings : uses
ScientificKeyPad <.. Settings : uses
BasicKeypad <.. Settings : uses
Class diagram for Utils formatting with regional symbolsclassDiagram
class Utils {
+QString formatThousandsSeparators(const QString &str)
+QString formatThousandsSeparatorsPro(const QString &str, const int Base)
}
Utils <.. Settings : uses
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:
- There’s a lot of repeated locale‐normalization logic scattered across files—consider refactoring it into a shared utility to reduce duplication and simplify maintenance.
- You’re creating a QDBusInterface and querying properties on every call; it would be more efficient to cache the interface or the retrieved symbols rather than re‐instantiating every time.
- The PR introduces many qInfo()/qDebug() logs; please review and remove or guard any excessive logging before merging to avoid performance and noise issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of repeated locale‐normalization logic scattered across files—consider refactoring it into a shared utility to reduce duplication and simplify maintenance.
- You’re creating a QDBusInterface and querying properties on every call; it would be more efficient to cache the interface or the retrieved symbols rather than re‐instantiating every time.
- The PR introduces many qInfo()/qDebug() logs; please review and remove or guard any excessive logging before merging to avoid performance and noise issues.
## Individual Comments
### Comment 1
<location> `src/widgets/expressionbar.cpp:1009` </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 (bug_risk):** Use of Unicode control characters as placeholders may cause confusion.
Using Unicode control characters as placeholders may cause bugs or display issues if processed or rendered externally. Consider a more explicit placeholder unlikely to conflict with user input or locale settings.
Suggested implementation:
```cpp
const QString decimalPlaceholder = QStringLiteral("__DECIMAL__");
```
```cpp
if (!decSym.isEmpty() && decSym != QLatin1String("."))
t.replace(decSym, decimalPlaceholder);
if (!grpSym.isEmpty() && grpSym != decSym)
t.replace(grpSym, "");
if (!decSym.isEmpty() && decSym != QLatin1String("."))
t.replace(decimalPlaceholder, QLatin1String("."));
```
</issue_to_address>
### Comment 2
<location> `src/widgets/inputedit.cpp:549` </location>
<code_context>
+ const QString allowedStatic = QString::fromUtf8("＋－×÷.,%()/");
+ QString filtered;
+ filtered.reserve(reformatStr.size());
+ for (const QChar &ch : std::as_const(reformatStr)) {
+ if (ch.isDigit() || ch.isLetter() || allowedStatic.contains(ch) || decSym.contains(ch) || grpSym.contains(ch)) {
+ filtered.append(ch);
</code_context>
<issue_to_address>
**issue:** Filtering logic may exclude valid locale-specific symbols.
The current logic may fail for multi-character symbols used in some locales. Please update the filtering to handle such cases or adopt a locale-aware method.
</issue_to_address>
### Comment 3
<location> `3rdparty/core/settings.cpp:405-410` </location>
<code_context>
+QString Settings::getSystemDecimalSymbol() const
+{
+ // 尝试从 D-Bus 获取控制中心设置
+ QDBusInterface interface("org.deepin.dde.Timedate1",
+ "/org/deepin/dde/Format1",
+ "org.deepin.dde.Format1",
+ QDBusConnection::sessionBus());
+
+ if (interface.isValid()) {
+ QVariant variant = interface.property("DecimalSymbol");
+ QString result = qvariant_cast<QString>(variant);
</code_context>
<issue_to_address>
**suggestion:** No error handling for D-Bus failures or timeouts.
Please add logging or error reporting for D-Bus failures to improve visibility and debugging.
</issue_to_address>
### Comment 4
<location> `src/widgets/expressionbar.cpp:1002` </location>
<code_context>
*/
QString ExpressionBar::formatExpression(const QString &text)
{
- return QString(text)
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting locale symbol conversion logic into two helper methods to centralize repeated code and improve readability.
Here are a few straightforward extractions that will collapse all of this repeated “get settings → swap in placeholders → strip/insert separators → swap back” logic into two self-documenting routines and make each of your core methods far easier to read and maintain.
1. Add in `expressionbar.h`:
```cpp
private:
QString toInternalLocale(const QString &text) const;
QString toDisplayLocale(const QString &text) const;
```
2. In `expressionbar.cpp`, implement them once:
```cpp
QString ExpressionBar::toInternalLocale(const QString &text) const
{
const auto sys = Settings::instance();
QString grp = sys->getSystemDigitGroupingSymbol();
QString dec = sys->getSystemDecimalSymbol();
// strip grouping, map decimal to '.'
QString t = text;
if (!grp.isEmpty() && grp != ",")
t.replace(grp, "");
if (!dec.isEmpty() && dec != ".")
t.replace(dec, ".");
return t;
}
QString ExpressionBar::toDisplayLocale(const QString &text) const
{
const auto sys = Settings::instance();
QString grp = sys->getSystemDigitGroupingSymbol();
QString dec = sys->getSystemDecimalSymbol();
QString t = text;
// first map '.' → placeholder → eventually back to localized decimal
QChar DEC_PH(0x1D), GRP_PH(0x1C);
t.replace(dec == "." ? QString() : dec, DEC_PH);
t.replace(grp == "," ? QString() : grp, GRP_PH);
t.replace(".", grp.isEmpty() ? "." : grp);
t.replace(",", dec.isEmpty() ? "." : dec);
t.replace(DEC_PH, dec.isEmpty() ? "." : dec);
t.replace(GRP_PH, grp.isEmpty() ? "," : grp);
return t;
}
```
3. Then simplify each method down to the essentials. For example **formatExpression** becomes:
```cpp
QString ExpressionBar::formatExpression(const QString &text)
{
// convert locale → internal symbols, strip grouping, then map operators
QString expr = toInternalLocale(text);
return expr.replace(QString::fromUtf8("＋"), "+")
.replace(QString::fromUtf8("－"), "-")
.replace(QString::fromUtf8("×"), "*")
.replace(QString::fromUtf8("÷"), "/")
.replace(QString::fromUtf8(","), "");
}
```
And **expressionCheck** roughly:
```cpp
void ExpressionBar::expressionCheck()
{
QString internal = toInternalLocale(m_inputEdit->text());
// ... do your fault-tolerance/zero-padding logic on `internal` ...
QString display = toDisplayLocale(internal);
m_inputEdit->setText(display);
// adjust cursor…
}
```
4. In **enterPointEvent**, you can now do:
```cpp
const auto decSym = Settings::instance()->getSystemDecimalSymbol();
const QString dot = decSym.isEmpty() ? "." : decSym;
// …use `toInternalLocale` or `toDisplayLocale` around your insertion logic…
```
By centralizing the locale‐symbol mapping you remove dozens of lines of near‐identical code from each of the four methods, and you keep each method focused on its own core job: inserting a point, fault-tolerance, formatting, or checking.
</issue_to_address>
### Comment 5
<location> `src/widgets/inputedit.cpp:466` </location>
<code_context>
void InputEdit::handleTextChanged(const QString &text)
{
qDebug() << "handleTextChanged called, text:" << text;
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the normalization, symbol-mapping, and filtering logic into dedicated helper functions to simplify handleTextChanged and formatExpression.
Here’s one way to collapse all of that inline normalization/reformatting/filtering into a small set of helpers, then call them from handleTextChanged (and formatExpression). You keep 100% of the behavior but handleTextChanged becomes a few calls instead of 100+ lines of fiddly `replace()`/loops.
```cpp
// InputFormatter.h
#pragma once
namespace InputFormatter {
// 1) normalize all locale‐specific symbols to “.” and strip grouping
QString normalizeLocale(const QString &input);
// 2) apply your +→＋, -→－ etc. mappings
QString sanitizeSymbols(const QString &input);
// 3) drop any character not in your allowed set
QString filterAllowed(const QString &input);
}
```
```cpp
// InputFormatter.cpp
#include "InputFormatter.h"
#include "../dsettings.h"
QString InputFormatter::normalizeLocale(const QString &expr) {
auto sys = Settings::instance();
QString s = expr;
const QString dec = sys->getSystemDecimalSymbol();
const QString grp = sys->getSystemDigitGroupingSymbol();
const QChar placeholder(0x1D);
if (!dec.isEmpty() && dec != ".")
s.replace(dec, placeholder);
if (!grp.isEmpty() && grp != dec)
s.remove(grp);
if (!dec.isEmpty() && dec != ".")
s.replace(placeholder, ".");
return s;
}
QString InputFormatter::sanitizeSymbols(const QString &input) {
static const std::vector<std::pair<QString, QString>> reps = {
{ "+", QString::fromUtf8("＋") },
{ "-", QString::fromUtf8("－") },
{ "_", QString::fromUtf8("－") },
{ "*", QString::fromUtf8("×") },
{ QString::fromUtf8("＊"), QString::fromUtf8("×") },
{ "X", QString::fromUtf8("×") },
{ QString::fromUtf8("（"), "(" },
{ QString::fromUtf8("）"), ")" },
{ QString::fromUtf8("——"), QString::fromUtf8("－") },
{ QString::fromUtf8("％"), "%" }
};
QString s = input;
for (auto &p : reps)
s.replace(p.first, p.second);
return s;
}
QString InputFormatter::filterAllowed(const QString &input) {
auto sys = Settings::instance();
QString allowed = QString::fromUtf8("＋－×÷.,%()/");
QString dec = sys->getSystemDecimalSymbol();
QString grp = sys->getSystemDigitGroupingSymbol();
QString out;
out.reserve(input.size());
for (QChar c : input) {
if (c.isDigit() || c.isLetter()
|| allowed.contains(c)
|| dec.contains(c)
|| grp.contains(c))
{
out.append(c);
}
}
return out;
}
```
Then in your handleTextChanged():
```cpp
void InputEdit::handleTextChanged(const QString &text) {
// … keep the cursor‐/ans‐logic above …
QString expr = InputFormatter::normalizeLocale(text);
QString reformatted = (Settings::instance()->programmerBase == 0)
? Utils::reformatSeparators(expr)
: Utils::reformatSeparatorsPro(expr, Settings::instance()->programmerBase);
reformatted = InputFormatter::sanitizeSymbols(reformatted);
multipleArithmetic(reformatted);
reformatted = InputFormatter::filterAllowed(reformatted);
if (reformatted != text) {
QSignalBlocker blocker(this);
setText(reformatted);
}
// … rest of cursor positioning, m_oldText, etc. …
}
```
And in formatExpression() you can also just:
```cpp
QString InputEdit::formatExpression(int probase, const QString &text) {
QString expr = InputFormatter::normalizeLocale(text);
// strip your UI symbols back to ascii operators:
expr.replace(QString::fromUtf8("＋"), "+")
.replace(QString::fromUtf8("－"), "-")
// … etc …
// then the rest of your base‐conversion switch…
return expr;
}
```
This pulls all of the localization, symbol-mapping and filtering into three tiny, testable functions and makes both handleTextChanged and formatExpression far shorter and easier to scan.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const QString grpSym = sys->getSystemDigitGroupingSymbol(); | ||
| const QString decimalPlaceholder = QString(QChar(0x1D)); | ||
|
|
||
| if (!decSym.isEmpty() && decSym != QLatin1String(".")) |
There was a problem hiding this comment.
suggestion (bug_risk): Use of Unicode control characters as placeholders may cause confusion.
Using Unicode control characters as placeholders may cause bugs or display issues if processed or rendered externally. Consider a more explicit placeholder unlikely to conflict with user input or locale settings.
Suggested implementation:
const QString decimalPlaceholder = QStringLiteral("__DECIMAL__");
if (!decSym.isEmpty() && decSym != QLatin1String("."))
t.replace(decSym, decimalPlaceholder);
if (!grpSym.isEmpty() && grpSym != decSym)
t.replace(grpSym, "");
if (!decSym.isEmpty() && decSym != QLatin1String("."))
t.replace(decimalPlaceholder, QLatin1String("."));
| const QString allowedStatic = QString::fromUtf8("＋－×÷.,%()/"); | ||
| QString filtered; | ||
| filtered.reserve(reformatStr.size()); | ||
| for (const QChar &ch : std::as_const(reformatStr)) { |
There was a problem hiding this comment.
issue: Filtering logic may exclude valid locale-specific symbols.
The current logic may fail for multi-character symbols used in some locales. Please update the filtering to handle such cases or adopt a locale-aware method.
JWWTSL
force-pushed
the
master
branch
2 times, most recently
from
42244b0 to
3d36edb
Compare
|
CLA Assistant Lite bot: You can retrigger this bot by commenting recheck in this Pull Request |
log: The calculator's regional format is currently out of sync with the Control Center's regional settings. This causes users in regions like Portuguese overseas territories to see a comma instead of a period as the thousands separator when using the calculator. task: https://pms.uniontech.com/story-view-39605.html
deepin pr auto review我将对这个diff进行详细的代码审查：
// 在settings.h中
class Settings {
// ...
static QString getSystemDecimalSymbol();
static QString getSystemDigitGroupingSymbol();
static bool getSystemDigitGrouping();
// ...
};
// 创建新的工具类LocaleHelper
class LocaleHelper {
public:
static QString normalizeGroupingSymbol(const QString &symbol);
static QString convertToInternalFormat(const QString &input, const QString &decSym, const QString &grpSym);
static QString convertToDisplayFormat(const QString &input, const QString &decSym, const QString &grpSym);
};
// 在settings.cpp中
QString Settings::getSystemDecimalSymbol()
{
try {
QDBusInterface interface("org.deepin.dde.Timedate1",
"/org/deepin/dde/Format1",
"org.deepin.dde.Format1",
QDBusConnection::sessionBus());
if (interface.isValid()) {
QVariant variant = interface.property("DecimalSymbol");
QString result = qvariant_cast<QString>(variant);
if (!result.isEmpty()) {
return result;
}
}
} catch (const std::exception &e) {
qWarning() << "Failed to get decimal symbol from D-Bus:" << e.what();
}
return QLocale().decimalPoint();
}
// 在formatThousandsSeparators中使用缓存
QString Utils::formatThousandsSeparators(const QString &str)
{
static const auto sys = Settings::instance();
static const QString decSym = sys->getSystemDecimalSymbol();
static const QString grpSym = sys->getSystemDigitGroupingSymbol();
static const bool groupingEnabled = sys->getSystemDigitGrouping();
// 使用缓存的值进行处理...
}
这些改进将使代码更加健壮、可维护，并且性能更好。 |
lzwind
approved these changes
Oct 30, 2025
lzwind
approved these changes
Oct 30, 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) |
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: The calculator's regional format is currently out of sync with the Control Center's regional settings. This causes users in regions like Portuguese overseas territories to see a comma instead of a period as the thousands separator when using the calculator.
task: https://pms.uniontech.com/story-view-39605.html
Summary by Sourcery
Synchronize calculator formatting with system regional settings by retrieving decimal and grouping symbols from Control Center, updating all relevant UI components and formatting logic to use dynamic separators, and bumping the package version.
New Features:
Enhancements:
Build: