fix: use desktop-id as fallback name for ill-formed desktop entries#703
Conversation
针对一些不规范的 desktop 文件,例如没有提供当前地区的显示名称,也没有 提供默认(无地区)显示名称的情况下,fallback到使用原始 desktop id 作为 对用户呈现的名称. PMS: BUG-349847 Log:
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a fallback so that for ill-formed desktop entries without a valid display name, the application uses the desktop-id as the display name instead. Flow diagram for AppItem displayName fallback to desktop idflowchart TD
A[Receive DBus appInfo for application] --> B[Create AppItem with
id from DesktopId
initial displayName from Name
initial genericName from GenericName]
B --> C{Is initial displayName empty?}
C -- Yes --> D[Set displayName to id]
C -- No --> E[Keep existing displayName]
D --> F{Is a localized Name field available?}
E --> F
F -- Yes --> G[Parse localized Name and assign to item name]
F -- No --> H[Skip name localization]
G --> I[Return AppItem to caller]
H --> I[Return AppItem to caller]
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
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/ddeintegration/appmgr.cpp:97-100` </location>
<code_context>
parseDBusField<QStringMap>(appInfo, u8"Name").value(),
parseDBusField<QStringMap>(appInfo, u8"GenericName").value());
+ // just in case the entry is ill-formed, doesn't have a valid display name, fallback to use its desktop-id instead.
+ if (item->displayName.isEmpty()) {
+ item->displayName = item->id;
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Treat whitespace-only display names as empty before applying the desktop-id fallback.
Because this uses `isEmpty()`, a display name containing only spaces or other whitespace will not fall back and will likely render poorly. Please use `item->displayName.trimmed().isEmpty()` so whitespace-only values also trigger the desktop-id fallback.
```suggestion
// just in case the entry is ill-formed, doesn't have a valid display name (or only whitespace),
// fallback to use its desktop-id instead.
if (item->displayName.trimmed().isEmpty()) {
item->displayName = item->id;
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // just in case the entry is ill-formed, doesn't have a valid display name, fallback to use its desktop-id instead. | ||
| if (item->displayName.isEmpty()) { | ||
| item->displayName = item->id; | ||
| } |
There was a problem hiding this comment.
suggestion: Treat whitespace-only display names as empty before applying the desktop-id fallback.
Because this uses isEmpty(), a display name containing only spaces or other whitespace will not fall back and will likely render poorly. Please use item->displayName.trimmed().isEmpty() so whitespace-only values also trigger the desktop-id fallback.
| // just in case the entry is ill-formed, doesn't have a valid display name, fallback to use its desktop-id instead. | |
| if (item->displayName.isEmpty()) { | |
| item->displayName = item->id; | |
| } | |
| // just in case the entry is ill-formed, doesn't have a valid display name (or only whitespace), | |
| // fallback to use its desktop-id instead. | |
| if (item->displayName.trimmed().isEmpty()) { | |
| item->displayName = item->id; | |
| } |
deepin pr auto review这段代码主要是在解析 DBus 返回的应用信息时,增加了一个容错处理逻辑:当应用没有有效的显示名称( 以下是从语法逻辑、代码质量、代码性能和代码安全四个方面的详细审查意见: 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进后的代码建议假设 static AppMgr::AppItem *parseDBus2AppItem(const ObjectInterfaceMap &source)
{
// ... (前置代码)
// 提前解析 "Name" 字段,避免重复调用
auto nameValueOpt = parseDBusField<QStringMap>(appInfo, u8"Name");
// 初始化 displayName
QString displayNameStr = nameValueOpt.value_or(QStringMap{}).value(); // 或者根据实际返回类型调整
// 如果 parseDBusField 返回的是 Optional<QStringMap>,这里需要解包
// 初始化 item
auto *item = new AppMgr::AppItem(
parseDBusField<QString>(appInfo, u8"Id").value(),
displayNameStr, // 使用解析出的值
parseDBusField<QStringMap>(appInfo, u8"GenericName").value());
// 容错处理:如果 display name 为空,回退到 ID
if (item->displayName.isEmpty()) {
item->displayName = item->id;
}
// 处理 name 字段
if (nameValueOpt) {
item->name = parseName(nameValueOpt.value());
}
// ... (后续代码)
}注意:由于 如果 总结原代码在功能上是可以工作的,但存在重复解析和逻辑不够紧凑的问题。通过提取公共变量和优化逻辑顺序,可以提高代码的可读性和维护性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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 |
针对一些不规范的 desktop 文件,例如没有提供当前地区的显示名称,也没有
提供默认(无地区)显示名称的情况下,fallback到使用原始 desktop id 作为
对用户呈现的名称.
PMS: BUG-349847
Summary by Sourcery
Bug Fixes: