Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add left/right padding to the main window #360

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hualet
Copy link
Contributor

@hualet hualet commented Sep 20, 2024

add left/right padding to the main window, or it looks like the term content is cut.

add left/right padding to the main window, or it looks like the
term content is cut.
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hualet

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

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

@deepin-ci-robot
Copy link

deepin pr auto review

关键摘要:

  • QTermWidget::backgroundColor函数中,当颜色表为空时返回了一个默认颜色,但没有检查background_color是否有效。如果table[DEFAULT_BACK_COLOR]没有有效的颜色设置,应该抛出一个异常或返回一个无效的颜色。
  • updateBackgroundColor函数中,直接使用了Settings::instance()->opacity()来设置背景颜色透明度,但没有检查这个值是否在有效范围内。
  • TermWidgetPage::updateBackgroundColor函数中,直接设置了this->setPalette(palette);this->setAutoFillBackground(true);,但没有检查palette是否为空或color是否有效。
  • TermWidgetPage::updateBackgroundColor函数中,使用了setAutoFillBackground(true);,这可能会导致性能问题,因为它会填充整个窗口的背景。应该考虑是否有更高效的方式来设置背景。
  • TermWidgetPage::updateBackgroundColor函数中,没有处理currentTerminal()为空的情况,这可能会导致未定义的行为。

是否建议立即修改:

void setColorScheme(const QString &name);


void updateBackgroundColor();
Copy link
Contributor

Choose a reason for hiding this comment

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

得用这个替换setColorScheme才能实现是吗?

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