fix: Change the app order in Privacy and Security#2825
Conversation
Reviewer's GuideThis PR refactors the app ordering mechanism in Privacy and Security by enhancing the sort field to include a category prefix (digits, letters, others) and updating the model insertion logic to maintain sorted order based on that sort key. Class diagram for updated ApplicationItem and AppsModelclassDiagram
class ApplicationItem {
- QString m_name
- QString m_sortField
+ void onNameChanged(const QString &name)
+ QString sortField()
}
class AppsModel {
- QList<ApplicationItem*> m_appItems
+ void appendItem(ApplicationItem *appItem)
}
ApplicationItem <.. AppsModel : uses
Flow diagram for new app insertion logic in AppsModelflowchart TD
A["New ApplicationItem created"] --> B["Calculate sortField (category + pinyin)"]
B --> C["Find insert position in m_appItems based on sortField"]
C --> D["Insert ApplicationItem at correct position"]
D --> E["App list remains sorted"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- appendItem uses a linear search for each insertion, leading to O(n²) behavior when resetting; consider sorting once and doing a bulk insert or using std::lower_bound for more efficient insertion.
- Extract the category-plus-pinyin sortField construction into a shared helper so the sorting logic stays consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- appendItem uses a linear search for each insertion, leading to O(n²) behavior when resetting; consider sorting once and doing a bulk insert or using std::lower_bound for more efficient insertion.
- Extract the category-plus-pinyin sortField construction into a shared helper so the sorting logic stays consistent and easier to maintain.
## Individual Comments
### Comment 1
<location> `src/plugin-privacy/operation/applicationitem.cpp:120` </location>
<code_context>
+ break;
+ }
+
+ QString pinyin = DTK_CORE_NAMESPACE::Chinese2Pinyin(upperName);
+ m_sortField = QString::number(category) + pinyin;
emitDataChanged();
}
</code_context>
<issue_to_address>
**suggestion:** Consider adding a separator between category and pinyin for sortField.
Directly joining category and pinyin may cause ambiguity if pinyin begins with a digit. Adding a separator, such as an underscore, will clarify the sortField format.
```suggestion
m_sortField = QString::number(category) + "_" + pinyin;
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
|
|
||
| QString pinyin = DTK_CORE_NAMESPACE::Chinese2Pinyin(upperName); | ||
| m_sortField = QString::number(category) + pinyin; |
There was a problem hiding this comment.
suggestion: Consider adding a separator between category and pinyin for sortField.
Directly joining category and pinyin may cause ambiguity if pinyin begins with a digit. Adding a separator, such as an underscore, will clarify the sortField format.
| m_sortField = QString::number(category) + pinyin; | |
| m_sortField = QString::number(category) + "_" + pinyin; |
|
TAG Bot New tag: 6.1.59 |
|
TAG Bot New tag: 6.1.60 |
|
TAG Bot New tag: 6.1.61 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, pengfeixx 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 |
|
TAG Bot New tag: 6.1.62 |
|
TAG Bot New tag: 6.1.63 |
Change the app order in Privacy and Security Log: Change the app order in Privacy and Security pms: BUG-303417
4fd7325 to
9d0dbfd
Compare
deepin pr auto review我来对这段代码的修改进行审查:
if (m_name == name) {
return;
}
int left = 0, right = m_appItems.size();
while (left < right) {
int mid = (left + right) / 2;
if (appItem->sortField() < m_appItems.at(mid)->sortField()) {
right = mid;
} else {
left = mid + 1;
}
}
insertPos = left;
if (name.isEmpty()) {
return;
}
int getCategory(const QString& name) {
QString upperName = name.toUpper();
for (const QChar &ch : upperName) {
if (ch.isSpace()) continue;
if (ch.isDigit()) return 0;
if (ch.isLetter() && ch.unicode() < 128) return 1;
return 2;
}
return 2;
}总体来说,这个修改是合理的,提高了应用列表的排序效果,但还可以在性能和代码组织方面做进一步优化。 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Change the app order in Privacy and Security
Log: Change the app order in Privacy and Security
pms: BUG-303417
Summary by Sourcery
Fix app ordering in Privacy and Security by categorizing names and inserting apps at the correct sorted position.
Bug Fixes: