Skip to content

fix(grub-theme): 调整 1024x768 分辨率下启动菜单宽度,修复显示不完整问题#190

Merged
mhduiy merged 1 commit intolinuxdeepin:masterfrom
zengwei00:master
May 9, 2026
Merged

fix(grub-theme): 调整 1024x768 分辨率下启动菜单宽度,修复显示不完整问题#190
mhduiy merged 1 commit intolinuxdeepin:masterfrom
zengwei00:master

Conversation

@zengwei00
Copy link
Copy Markdown

fix(grub-theme): 调整 1024x768 分辨率下启动菜单宽度,修复显示不完整问题

pms: 359281

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @zengwei00, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

TAG Bot

TAG: 6.0.41
EXISTED: no
DISTRIBUTION: unstable

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴,但您似乎并非合作伙伴的成员或对接人,请联系相关对接人将您添加至组织之中,或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

zengwei seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码变更主要针对 GRUB 主题在 1024x768 分辨率下的显示问题进行了修复。以下是对代码的详细审查和改进意见:

1. 语法逻辑审查

main.go 部分:

  • 修改内容:将 halfWidthPercent22 修改为 27
  • 逻辑分析
    • 原逻辑:宽度 = 22 * 2 = 44%,左边距 = 50% - 22% = 28%。
    • 新逻辑:宽度 = 27 * 2 = 54%,左边距 = 50% - 27% = 23%。
    • 结论:修改后启动菜单宽度从 44% 增加到 54%,左边距从 28% 减少到 23%,使菜单更宽且居中。这确实可以解决显示不完整的问题,逻辑合理。

version.go 部分:

  • 修改内容:版本号从 19 更新为 20
  • 结论:版本号递增符合规范。

changelog 部分:

  • 修改内容:添加了版本 6.0.41 的更新日志,描述了修复内容。
  • 结论:日志描述清晰,符合 Debian changelog 格式。

2. 代码质量审查

  • 硬编码问题halfWidthPercent 的值是硬编码的,仅针对 1024x768 分辨率。如果未来需要支持其他分辨率,可能需要扩展逻辑。
  • 可读性:代码注释清晰,解释了 halfWidthPercent 的含义和计算逻辑,值得肯定。

改进建议

// 建议将硬编码值提取为常量,提高可维护性
const (
    Resolution1024x768Width  = 1024
    Resolution1024x768Height = 768
    HalfWidthPercent1024x768 = 27 // 针对 1024x768 的半宽百分比
)

func adjustBootMenuV25(comp *tt.Component, width, height int) {
    if width == Resolution1024x768Width && height == Resolution1024x768Height {
        comp.SetProp("width", tt.RelNum(HalfWidthPercent1024x768*2))
        comp.SetProp("left", tt.RelNum(50-HalfWidthPercent1024x768))
    }
}

3. 代码性能审查

  • 性能影响:修改仅涉及简单的算术运算和属性设置,对性能无显著影响。
  • 优化建议:无需优化。

4. 代码安全审查

  • 安全风险
    • 无输入验证:widthheight 是外部传入的参数,未验证其合法性(如负值或超大值)。
    • 硬编码依赖:如果 tt.RelNum 的行为或参数范围发生变化,可能导致布局异常。

改进建议

func adjustBootMenuV25(comp *tt.Component, width, height int) {
    // 添加输入验证
    if width <= 0 || height <= 0 {
        return
    }

    if width == Resolution1024x768Width && height == Resolution1024x768Height {
        halfWidthPercent := HalfWidthPercent1024x768
        // 确保计算结果在合理范围内
        if halfWidthPercent*2 > 100 || halfWidthPercent > 50 {
            return // 避免无效的布局参数
        }
        comp.SetProp("width", tt.RelNum(halfWidthPercent*2))
        comp.SetProp("left", tt.RelNum(50-halfWidthPercent))
    }
}

5. 其他建议

  • 多分辨率支持:如果未来需要支持更多分辨率,建议使用映射表或配置文件:
    var resolutionHalfWidthMap = map[string]int{
        "1024x768": 27,
        "1920x1080": 30, // 示例
    }
    
    func getHalfWidthPercent(width, height int) int {
        key := fmt.Sprintf("%dx%d", width, height)
        if percent, ok := resolutionHalfWidthMap[key]; ok {
            return percent
        }
        return 22 // 默认值
    }
  • 测试覆盖:建议为 adjustBootMenuV25 添加单元测试,覆盖以下场景:
    • 1024x768 分辨率下的布局计算。
    • 非目标分辨率下的行为。
    • 边界值(如 widthheight 为 0 或负数)。

总结

  • 优点:修复了显示问题,逻辑清晰,注释完善。
  • 缺点:硬编码值较多,缺乏输入验证和多分辨率扩展性。
  • 推荐行动
    1. 提取硬编码值为常量。
    2. 添加输入验证。
    3. 考虑多分辨率支持的扩展设计。
    4. 补充单元测试。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy, zengwei00

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mhduiy mhduiy merged commit 2570baf into linuxdeepin:master May 9, 2026
18 of 19 checks passed
@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 9, 2026

TAG Bot

Tag created successfully

📋 Tag Details
  • Tag Name: 6.0.41
  • Tag SHA: edaf9a541120d3ab83a20f3c87c3905fd049f397
  • Commit SHA: cefd0ceb8860bb7c3daf48dbfb5bc1b4a1924bd6
  • Tag Message:
    Release dde-api 6.0.41
    
    
  • Tagger:
    • Name: zengwei00
  • Distribution: unstable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants