Feat: Add group setting to show screen size.#630
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Mar 31, 2026
Merged
Feat: Add group setting to show screen size.#630deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
-- Add group setting to show screen size. Log: add feature Task: https://pms.uniontech.com/task-view-387965.html
Reviewer's GuideRefactors screen size visibility into a reusable Common::isShowScreenSize() helper and applies it to both monitor overview text formatting and the "Size" field display, with behavior dependent on a configuration flag and special device types. Sequence diagram for screen size visibility decision using Common_isShowScreenSizesequenceDiagram
participant DeviceMonitor
participant Common
participant DConfig
DeviceMonitor->>Common: isShowScreenSize()
activate Common
Common->>DConfig: create(org.deepin.devicemanager, org.deepin.devicemanager)
activate DConfig
DConfig-->>Common: dconfig
deactivate DConfig
alt DConfig_valid_and_has_showScreenSize
Common->>DConfig: isValid()
DConfig-->>Common: true
Common->>DConfig: keyList()
DConfig-->>Common: keys_with_showScreenSize
Common->>DConfig: value(showScreenSize)
DConfig-->>Common: bool
Common-->>DeviceMonitor: showScreenSize_flag
else DConfig_invalid_or_missing_key
Common-->>DeviceMonitor: true
end
deactivate Common
Note over DeviceMonitor,Common: DeviceMonitor then uses the flag to decide whether to show screen size in overview and Size field
Sequence diagram for adding Size field in loadOtherDeviceInfo based on Common_isShowScreenSizesequenceDiagram
participant DeviceMonitor
participant Common
DeviceMonitor->>DeviceMonitor: addOtherDeviceInfo(Primary_Monitor, m_MainScreen)
DeviceMonitor->>Common: isShowScreenSize()
Common-->>DeviceMonitor: showScreenSize_flag
alt showScreenSize_flag_true
DeviceMonitor->>DeviceMonitor: addOtherDeviceInfo(Size, m_ScreenSize)
else showScreenSize_flag_false
DeviceMonitor->>DeviceMonitor: skip_Size_field
end
DeviceMonitor->>DeviceMonitor: addOtherDeviceInfo(Serial_Number, m_SerialNumber)
Updated class diagram for Common_and_DeviceMonitor_screen_size_logicclassDiagram
class Common {
<<static>> int specialComType
<<static>> QByteArray executeClientCmd(QString cmd, QStringList args, QString workPath, int msecsWaiting, bool useEnv)
<<static>> bool isShowScreenSize()
}
class DeviceMonitor {
QString m_Name
QString m_ScreenSize
QString m_MainScreen
QString m_SerialNumber
QString getOverviewInfo()
void loadOtherDeviceInfo()
}
class DConfig {
static DConfig* create(QString name, QString applicationId)
bool isValid()
QStringList keyList()
QVariant value(QString key)
}
DeviceMonitor ..> Common : uses
Common ..> DConfig : reads_showScreenSize_config
Flow diagram for monitor overview string based on screen size settingflowchart TD
A[Start getOverviewInfo] --> B[Call Common_isShowScreenSize]
B --> C{isShowScreenSize == true}
C -->|true| D{specialComType == 6 or 7}
D -->|true| E[Set ov to m_ScreenSize]
D -->|false| F[Set ov to m_Name of m_ScreenSize]
C -->|false| G{specialComType == 6 or 7}
G -->|true| H[Set ov to empty]
G -->|false| I[Set ov to m_Name]
E --> J[Return ov]
F --> J
H --> J
I --> J
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 - I've found 2 issues, and left some high level feedback:
- Common::isShowScreenSize() creates a DConfig instance on every call without managing its lifetime; consider reusing a shared DConfig instance or ensuring it is properly destroyed to avoid repeated allocations or potential leaks.
- The branching in DeviceMonitor::getOverviewInfo() for specialComType 6/7 vs others is now duplicated in both the isShowScreenSize() and !isShowScreenSize() paths; consider refactoring this into a small helper or more compact conditional to reduce repetition and make the logic easier to follow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Common::isShowScreenSize() creates a DConfig instance on every call without managing its lifetime; consider reusing a shared DConfig instance or ensuring it is properly destroyed to avoid repeated allocations or potential leaks.
- The branching in DeviceMonitor::getOverviewInfo() for specialComType 6/7 vs others is now duplicated in both the isShowScreenSize() and !isShowScreenSize() paths; consider refactoring this into a small helper or more compact conditional to reduce repetition and make the logic easier to follow.
## Individual Comments
### Comment 1
<location path="deepin-devicemanager/src/commonfunction.cpp" line_range="307-308" />
<code_context>
addOtherDeviceInfo("Primary Monitor", m_MainScreen);
- bool showScreenSize { true };
-#ifdef DTKCORE_CLASS_DConfigFile
- DConfig *dconfig = DConfig::create("org.deepin.devicemanager","org.deepin.devicemanager");
- if(dconfig && dconfig->isValid() && dconfig->keyList().contains("showScreenSize")){
- showScreenSize = dconfig->value("showScreenSize").toBool();
- }
</code_context>
<issue_to_address>
**issue (bug_risk):** Clarify and enforce ownership of the DConfig instance to avoid potential leaks.
The raw pointer from `DConfig::create` isn’t freed here. Unless DTK guarantees ownership via a parent or other mechanism, this risks leaking on each call. Consider assigning a parent, wrapping it in a smart pointer, or reusing a shared instance if repeated calls are expected.
</issue_to_address>
### Comment 2
<location path="deepin-devicemanager/src/DeviceManager/DeviceMonitor.cpp" line_range="359-360" />
<code_context>
-
- if (Common::specialComType == 6 || Common::specialComType == 7) {
- ov = QString("(%1)").arg(m_ScreenSize);
+ if (Common::isShowScreenSize()) {
+ if (Common::specialComType == 6 || Common::specialComType == 7) {
+ ov = QString("(%1)").arg(m_ScreenSize);
+ } else {
</code_context>
<issue_to_address>
**suggestion:** Reduce duplication by extracting the repeated specialComType check.
The `Common::specialComType == 6 || Common::specialComType == 7` condition appears in both branches of the outer `if`. Extract it into a local `bool isSpecial = ...;` or a small helper to simplify the control flow and reduce future maintenance risk if special types change.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+307
to
+308
| DConfig *dconfig = DConfig::create("org.deepin.devicemanager","org.deepin.devicemanager"); | ||
| if(dconfig && dconfig->isValid() && dconfig->keyList().contains("showScreenSize")){ |
There was a problem hiding this comment.
issue (bug_risk): Clarify and enforce ownership of the DConfig instance to avoid potential leaks.
The raw pointer from DConfig::create isn’t freed here. Unless DTK guarantees ownership via a parent or other mechanism, this risks leaking on each call. Consider assigning a parent, wrapping it in a smart pointer, or reusing a shared instance if repeated calls are expected.
lzwind
approved these changes
Mar 31, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, 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
|
/merge |
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.
-- Add group setting to show screen size.
Log: add feature
Task: https://pms.uniontech.com/task-view-387965.html
Summary by Sourcery
Add configurable control for displaying monitor screen size in device manager views.
New Features:
Enhancements: