[PATCH 1 of 2] qtlib: indicate the correct button selection for CustomPrompt (fixes #5981)

14 views
Skip to first unread message

Matt Harbison

unread,
Jun 20, 2024, 3:48:55 PMJun 20
to thg...@googlegroups.com
# HG changeset patch
# User Matt Harbison <matt_h...@yahoo.com>
# Date 1718891935 14400
# Thu Jun 20 09:58:55 2024 -0400
# Branch stable
# Node ID 59f312da84aa0a9649974ea8843fd959dbad0c37
# Parent 188c9536046d38c611f8e1f45cc52605307c0990
qtlib: indicate the correct button selection for CustomPrompt (fixes #5981)

I didn't have a problem on Qt5 or Qt6 on Windows, but the documentation says
that the value returned from `.exec()` is opaque when using custom buttons[1],
so this looks like the correct way to do what the previous code was attempting.

[1] https://doc.qt.io/qt-6/qmessagebox.html#exec

diff --git a/tortoisehg/hgqt/qtlib.py b/tortoisehg/hgqt/qtlib.py
--- a/tortoisehg/hgqt/qtlib.py
+++ b/tortoisehg/hgqt/qtlib.py
@@ -941,7 +941,8 @@
self.setEscapeButton(btn)

def run(self) -> int:
- return self.exec()
+ self.exec()
+ return self.buttons().index(self.clickedButton())

def keyPressEvent(self, event: QKeyEvent) -> None:
for k, btn in self.hotkeys.items():

Matt Harbison

unread,
Jun 20, 2024, 3:48:57 PMJun 20
to thg...@googlegroups.com
# HG changeset patch
# User Matt Harbison <matt_h...@yahoo.com>
# Date 1718911575 14400
# Thu Jun 20 15:26:15 2024 -0400
# Branch stable
# Node ID a00cdef7241b79519593aa9549140205fe7cf4ad
# Parent 59f312da84aa0a9649974ea8843fd959dbad0c37
commit: correct the commit button width for Qt6 (fixes #5968)

I don't recall seeing the truncated button myself on Windows with Qt6, but maybe
I used an earlier version of Qt6. In any case, the button doesn't seem much
larger (if it is at all) on Windows with Qt5 with this change, and I couldn't
track down the reason for the magic multiplication from afe08b4e6fd6, nor what
it is trying to do here. Even with the MQ extension enabled, the button looks
fine on Qt5. Maybe it was trying to make the button narrower than the menu
(since the menu has longer text than "Commit" or "Amend"), but that's still the
case with this change.

diff --git a/tortoisehg/hgqt/commit.py b/tortoisehg/hgqt/commit.py
--- a/tortoisehg/hgqt/commit.py
+++ b/tortoisehg/hgqt/commit.py
@@ -504,7 +504,7 @@
committb.setPopupMode(QToolButton.ToolButtonPopupMode.MenuButtonPopup)
fmk = lambda s: committb.fontMetrics().horizontalAdvance(hglib.tounicode(s[2]))
committb._width = (max(pycompat.maplist(fmk, acts))
- + 4*committb.menuButtonWidth())
+ + committb.width())

class CommitButtonMenu(QMenu):
def __init__(self, parent, repo):

Matt Harbison

unread,
Jun 20, 2024, 4:17:32 PMJun 20
to TortoiseHg Developers
I guess we're due for a 6.7.4 tag, since 6.6.3 was the last release.

Yuya Nishihara

unread,
Jun 20, 2024, 11:28:28 PMJun 20
to Matt Harbison, thg...@googlegroups.com
Maybe safer to handle None explicitly (and return -1)? I'm not sure what would
happen if the dialog is closed without clicking custom buttons (e.g. by Escape.)

Yuya Nishihara

unread,
Jun 20, 2024, 11:28:31 PMJun 20
to Matt Harbison, thg...@googlegroups.com
.width() here doesn't make sense because:

* .width() at this point wouldn't return reasonable value because the widget
isn't laid out yet.
* this code appears to calculate margins around the label
Commit|Amend|QNew|QRefresh.

Suppose we don't care much about this hack, maybe we can add a constant margin
(say 10px + icon width if any?) and .setMinimumWidth()?

Matt Harbison

unread,
Jun 21, 2024, 12:18:47 AMJun 21
to TortoiseHg Developers
Good call.  It also returns None unless an escape button was set.  I missed that, and only saw the part about if .exec() wasn't called yet.


Matt Harbison

unread,
Jun 21, 2024, 12:19:15 AMJun 21
to thg...@googlegroups.com
# HG changeset patch
# User Matt Harbison <matt_h...@yahoo.com>
# Date 1718891935 14400
# Thu Jun 20 09:58:55 2024 -0400
# Branch stable
# Node ID 46fc857e6407ab104030f7eafb697e957afa0b7c
# Parent 188c9536046d38c611f8e1f45cc52605307c0990
qtlib: indicate the correct button selection for CustomPrompt (fixes #5981)

I didn't have a problem on Qt5 or Qt6 on Windows, but the documentation says
that the value returned from `.exec()` is opaque when using custom buttons[1],
so this looks like the correct way to do what the previous code was attempting.

[1] https://doc.qt.io/qt-6/qmessagebox.html#exec

diff --git a/tortoisehg/hgqt/qtlib.py b/tortoisehg/hgqt/qtlib.py
--- a/tortoisehg/hgqt/qtlib.py
+++ b/tortoisehg/hgqt/qtlib.py
@@ -941,7 +941,11 @@
self.setEscapeButton(btn)

def run(self) -> int:
- return self.exec()
+ self.exec()
+ clicked = self.clickedButton()
+ if clicked is None:
+ return -1
+ return self.buttons().index(clicked)

Matt Harbison

unread,
Jun 21, 2024, 12:28:37 AMJun 21
to TortoiseHg Developers
There's got to be some function on the widget that can provide that info without hard coding values like that, right?  It doesn't look like the button itself ever has icons, but the popup menu has a dot for the item selected (which shouldn't matter here).  Would QWidget::contentsMargins() do the trick?  The documentation doesn't say when that's initialized, so IDK if it starts with some platform L&F specific values.

 

Yuya Nishihara

unread,
Jun 21, 2024, 1:57:52 AMJun 21
to 'Matt Harbison' via TortoiseHg Developers
IIRC, contentsMargins() are the values to be used by layout of nested widgets
(such as (QLabel, QLineEdit) pairs in QFormLayout.) I don't think the interior
layout of the button would use these values, which usually respects the platform
style.

I think a hard-coded value is practically good to stabilize the button width.
The actual width might exceed the hard-coded value (if translated label is
super long), but it's okay so long as the button can stretch as needed. I think
.setMinimumWidth(100) or something works.

Yuya Nishihara

unread,
Jun 21, 2024, 1:57:54 AMJun 21
to Matt Harbison, thg...@googlegroups.com
On Fri, 21 Jun 2024 00:19:09 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_h...@yahoo.com>
> # Date 1718891935 14400
> # Thu Jun 20 09:58:55 2024 -0400
> # Branch stable
> # Node ID 46fc857e6407ab104030f7eafb697e957afa0b7c
> # Parent 188c9536046d38c611f8e1f45cc52605307c0990
> qtlib: indicate the correct button selection for CustomPrompt (fixes #5981)

Queued for stable, thanks.
Reply all
Reply to author
Forward
0 new messages