[PATCH 1 of 2] blockmatcher: add type hints to four functions in BlockList. Fix a type error

[Antonio Muci]

# HG changeset patch
# User Antonio Muci <a....@inwind.it>
# Date 1742332955 -3600
# Tue Mar 18 22:22:35 2025 +0100
# Branch stable
# Node ID 46987ac2df43ba55ad335690480c1c60e754d2ce
# Parent ae24d8b47e5dde57abecb97e33bf431a9688bdac
blockmatcher: add type hints to four functions in BlockList. Fix a type error

This commit adds type hints to the four functions that are involved in a Qt6
crash detailed in #6023. In the process it becomes evident a type mismatch that
always existed. The commit also fixes that one.

This is the mismatch: scrollToPos() called setValue() with a float value.
setValue() in turn emitted the pyqtSignal(int) valueChanged, but passing the
float value instead.

This was fine in Qt5, but would break in Qt6.

This commit puts in place the type hints and rounds to int "value" as a
preventive measure, that stays compatible with Qt5, and would not break under
Qt6.

The final fix for the crash will be put in place in the next commit.

diff --git a/tortoisehg/hgqt/blockmatcher.py b/tortoisehg/hgqt/blockmatcher.py
--- a/tortoisehg/hgqt/blockmatcher.py
+++ b/tortoisehg/hgqt/blockmatcher.py
@@ -92,7 +92,7 @@ class BlockList(QWidget):
self.update()
self.rangeChanged.emit(self._minimum, self._maximum)

- def setValue(self, val):
+ def setValue(self, val: int) -> None:
if val != self._value:
self._value = val
self.update()
@@ -141,21 +141,21 @@ class BlockList(QWidget):
p.setBrush(self._vrectcolor)
p.drawRect(0, int(self._value * sy), w, int(self._pagestep * sy))

- def scrollToPos(self, y):
+ def scrollToPos(self, y: int) -> None:
# Scroll to the position which specified by Y coodinate.
if not isinstance(self._sbar, QScrollBar):
return
ratio = float(y) / self.height()
minimum, maximum, step = self._minimum, self._maximum, self._pagestep
- value = minimum + (maximum + step - minimum) * ratio - (step * 0.5)
+ value = int(minimum + (maximum + step - minimum) * ratio - (step * 0.5))
value = min(maximum, max(minimum, value)) # round to valid range.
self.setValue(value)

- def mousePressEvent(self, event):
+ def mousePressEvent(self, event: QMouseEvent) -> None:
super().mousePressEvent(event)
self.scrollToPos(event.y())

- def mouseMoveEvent(self, event):
+ def mouseMoveEvent(self, event: QMouseEvent) -> None:
super().mouseMoveEvent(event)
self.scrollToPos(event.y())

[Antonio Muci]

# HG changeset patch
# User Antonio Muci <a....@inwind.it>

# Date 1742334305 -3600
# Tue Mar 18 22:45:05 2025 +0100
# Branch stable
# Node ID 7063b2d8df76ecab039ea7cbf703bac2bee00795
# Parent 46987ac2df43ba55ad335690480c1c60e754d2ce
blockmatcher: fix crash on Qt6 when clicking on the left minimap

This crash is described in
https://foss.heptapod.net/mercurial/tortoisehg/thg/-/issues/6023 and is due to
a deprecation in Qt 6.0 (https://doc.qt.io/qt-6/qmouseevent-obsolete.html#y).

We follow the advice given in that page, and use position().y() instead.

Since the y() function returns a float under Qt6, we have to cast it to int.

The commit retains a fallback path for Qt5 that can be removed if/when removing
support for Qt5.

Tested on Fedora 38/PyQt5 and Fedora 41/PyQt6.

Fixes #6023.

diff --git a/tortoisehg/hgqt/blockmatcher.py b/tortoisehg/hgqt/blockmatcher.py
--- a/tortoisehg/hgqt/blockmatcher.py
+++ b/tortoisehg/hgqt/blockmatcher.py

@@ -153,11 +153,45 @@ class BlockList(QWidget):

def mousePressEvent(self, event: QMouseEvent) -> None:
super().mousePressEvent(event)

- self.scrollToPos(event.y())
+ try:
+ # In Qt6, QMouseEvent.positon() returns a QPointF, whose x() and y()
+ # methods are floats. Since we need an int, we have to cast y().
+ #
+ # see: https://doc.qt.io/qt-6/qsinglepointevent.html#position
+ y_coord = int(event.position().y())
+ except AttributeError:
+ # fallback path for Qt5, to be removed once support for Qt5 is
+ # removed.
+ #
+ # In Qt5 QMouseEvent::y() returns an int (source: source:
+ # https://qthub.com/static/doc/qt5/qtgui/qmouseevent.html#y), and we
+ # need no cast.
+ #
+ # In Qt6 we will use position().y() as instructed in
+ # https://doc.qt.io/qt-6/qmouseevent-obsolete.html#y
+ y_coord = event.y()
+ self.scrollToPos(y_coord)

def mouseMoveEvent(self, event: QMouseEvent) -> None:
super().mouseMoveEvent(event)

- self.scrollToPos(event.y())
+ try:
+ # In Qt6, QMouseEvent.positon() returns a QPointF, whose x() and y()
+ # methods are floats. Since we need an int, we have to cast y().
+ #
+ # see: https://doc.qt.io/qt-6/qsinglepointevent.html#position
+ y_coord = int(event.position().y())
+ except AttributeError:
+ # fallback path for Qt5, to be removed once support for Qt5 is
+ # removed.
+ #
+ # In Qt5 QMouseEvent::y() returns an int (source: source:
+ # https://qthub.com/static/doc/qt5/qtgui/qmouseevent.html#y), and we
+ # need no cast.
+ #
+ # In Qt6 we will use position().y() as instructed in
+ # https://doc.qt.io/qt-6/qmouseevent-obsolete.html#y
+ y_coord = event.y()
+ self.scrollToPos(y_coord)

class BlockMatch(BlockList):
"""

[Matt Harbison]

On Tuesday, March 18, 2025 at 6:11:46 PM UTC-4 Antonio Muci wrote:

# HG changeset patch
# User Antonio Muci <a....@inwind.it>
# Date 1742334305 -3600
# Tue Mar 18 22:45:05 2025 +0100
# Branch stable
# Node ID 7063b2d8df76ecab039ea7cbf703bac2bee00795
# Parent 46987ac2df43ba55ad335690480c1c60e754d2ce
blockmatcher: fix crash on Qt6 when clicking on the left minimap

Fixes #6023.

If you append `(fixes #6023)` to the commit summary, it will auto close when this lands.

 

diff --git a/tortoisehg/hgqt/blockmatcher.py b/tortoisehg/hgqt/blockmatcher.py
--- a/tortoisehg/hgqt/blockmatcher.py
+++ b/tortoisehg/hgqt/blockmatcher.py
@@ -153,11 +153,45 @@ class BlockList(QWidget):

def mousePressEvent(self, event: QMouseEvent) -> None:
super().mousePressEvent(event)
- self.scrollToPos(event.y())
+ try:
+ # In Qt6, QMouseEvent.positon() returns a QPointF, whose x() and y()
+ # methods are floats. Since we need an int, we have to cast y().
+ #
+ # see: https://doc.qt.io/qt-6/qsinglepointevent.html#position
+ y_coord = int(event.position().y())
+ except AttributeError:
+ # fallback path for Qt5, to be removed once support for Qt5 is
+ # removed.
+ #
+ # In Qt5 QMouseEvent::y() returns an int (source: source:
+ # https://qthub.com/static/doc/qt5/qtgui/qmouseevent.html#y), and we
+ # need no cast.
+ #
+ # In Qt6 we will use position().y() as instructed in
+ # https://doc.qt.io/qt-6/qmouseevent-obsolete.html#y
+ y_coord = event.y()

Maybe put this in qtlib, near the very similar `qDropEventPosition()`.  (Too bad these are unrelated event classes, as it seems the logic is almost identical.)

[Yuya Nishihara]

On Tue, 18 Mar 2025 23:11:41 +0100, 'Antonio Muci' via TortoiseHg
Developers wrote:
> # HG changeset patch
> # User Antonio Muci <a....@inwind.it>
> # Date 1742332955 -3600
> # Tue Mar 18 22:22:35 2025 +0100
> # Branch stable
> # Node ID 46987ac2df43ba55ad335690480c1c60e754d2ce
> # Parent ae24d8b47e5dde57abecb97e33bf431a9688bdac
> blockmatcher: add type hints to four functions in BlockList. Fix a
> type error

Queued the first patch, thanks.

[Antonio Muci]

# HG changeset patch
# User Antonio Muci <a....@inwind.it>

# Date 1742409163 -3600
# Wed Mar 19 19:32:43 2025 +0100
# Branch stable
# Node ID 7b4caae56aecd0ca69bbd0867f553dcb0a0c433b
# Parent cde4a42b1a2c6cc3f0ebccdb05df9e4a86986715

blockmatcher: fix crash on Qt6 when clicking on the left minimap

This crash is described in
https://foss.heptapod.net/mercurial/tortoisehg/thg/-/issues/6023 and is due to
a deprecation in Qt 6.0 (https://doc.qt.io/qt-6/qmouseevent-obsolete.html#y).

We follow the advice given in that page, and use position().y() instead.

Since the y() function returns a float under Qt6, we have to cast it to int.

The commit retains a fallback path for Qt5 that can be removed if/when removing
support for Qt5.

Tested on Fedora 38/PyQt5 and Fedora 41/PyQt6.

(fixes #6023)

diff --git a/tortoisehg/hgqt/blockmatcher.py b/tortoisehg/hgqt/blockmatcher.py
--- a/tortoisehg/hgqt/blockmatcher.py
+++ b/tortoisehg/hgqt/blockmatcher.py

@@ -36,6 +36,8 @@ from .qtgui import (
QWidget,
)

+from . import qtlib
+
class BlockList(QWidget):
"""
A simple widget to be 'linked' to the scrollbar of a diff text
@@ -153,11 +155,11 @@ class BlockList(QWidget):

def mousePressEvent(self, event: QMouseEvent) -> None:
super().mousePressEvent(event)
- self.scrollToPos(event.y())

+ self.scrollToPos(qtlib.qMouseEventY(event))

def mouseMoveEvent(self, event: QMouseEvent) -> None:
super().mouseMoveEvent(event)
- self.scrollToPos(event.y())

+ self.scrollToPos(qtlib.qMouseEventY(event))

class BlockMatch(BlockList):
"""
diff --git a/tortoisehg/hgqt/qtlib.py b/tortoisehg/hgqt/qtlib.py
--- a/tortoisehg/hgqt/qtlib.py
+++ b/tortoisehg/hgqt/qtlib.py
@@ -1679,3 +1679,21 @@ def qDropEventPosition(event: QDropEvent
return event.position().toPoint() # pytype: disable=attribute-error
else:
return event.pos() # pytype: disable=attribute-error
+
+
+def qMouseEventY(event: QMouseEvent) -> int:
+ """Return an int representing the y coordinate of a QMouseEvent"""
+ if QT_VERSION >= 0x060000:
+ # In Qt6, QMouseEvent.position() returns a QPointF, whose x() and y()

+ # methods are floats. Since we need an int, we have to cast y()

+ #
+ # see: https://doc.qt.io/qt-6/qsinglepointevent.html#position

+ return int(event.position().y()) # pytype: disable=attribute-error
+ else:
+ # In Qt5, QMouseEvent::y() exists and returns an int, and we need no
+ # cast.
+ #
+ # see:
+ # - https://qthub.com/static/doc/qt5/qtgui/qmouseevent.html#y
+ # - https://doc.qt.io/qt-6/qmouseevent-obsolete.html#y
+ return event.y()

[Yuya Nishihara]

On Wed, 19 Mar 2025 20:44:27 +0100, 'Antonio Muci' via TortoiseHg
Developers wrote:
> # HG changeset patch
> # User Antonio Muci <a....@inwind.it>
> # Date 1742409163 -3600
> # Wed Mar 19 19:32:43 2025 +0100
> # Branch stable
> # Node ID 7b4caae56aecd0ca69bbd0867f553dcb0a0c433b
> # Parent cde4a42b1a2c6cc3f0ebccdb05df9e4a86986715
> blockmatcher: fix crash on Qt6 when clicking on the left minimap

I think we can basically copy qDropEventPosition() for QMouseEvent.
Then, caller can do qMouseEventPosition(event).y()?

In Qt5, .y() is equivalent to .pos().y().

[a....@inwind.it]

> In Qt5, .y() is equivalent to .pos().y().

Almost: In Qt5 .y() returns an int. In Qt6 .position() returns a QPointF, whose y() returns a float.

I _think_ that now that in BlockList.scrollToPos() there is a cast to int, returning an int | float could work (barring the necessary hint changes).

However, before adding the cast, under Qt6 the scrolling was not working because we ended up emitting a valueChanged(float).

[Yuya Nishihara]

On Thu, 20 Mar 2025 13:55:40 +0100 (CET), a....@inwind.it wrote:
> > In Qt5, .y() is equivalent to .pos().y().
>
> Almost: In Qt5 .y() returns an int. In Qt6 .position() returns a
> QPointF, whose y() returns a float.

That's why we have .toPoint() in qDropEventPosition(), right?

[Antonio Muci]

# HG changeset patch
# User Antonio Muci <a....@inwind.it>

# Date 1742481008 -3600
# Thu Mar 20 15:30:08 2025 +0100
# Branch stable
# Node ID 07de97de21ffda32154867da7274ae1b80e9dd9b

# Parent cde4a42b1a2c6cc3f0ebccdb05df9e4a86986715
blockmatcher: fix crash on Qt6 when clicking on the left minimap

Tested on Fedora 38/PyQt5 and Fedora 41/PyQt6.

(fixes #6023)

diff --git a/tortoisehg/hgqt/blockmatcher.py b/tortoisehg/hgqt/blockmatcher.py
--- a/tortoisehg/hgqt/blockmatcher.py
+++ b/tortoisehg/hgqt/blockmatcher.py
@@ -36,6 +36,8 @@ from .qtgui import (
QWidget,
)

+from . import qtlib
+
class BlockList(QWidget):
"""
A simple widget to be 'linked' to the scrollbar of a diff text
@@ -153,11 +155,11 @@ class BlockList(QWidget):

def mousePressEvent(self, event: QMouseEvent) -> None:
super().mousePressEvent(event)
- self.scrollToPos(event.y())

+ self.scrollToPos(qtlib.qMouseEventPosition(event).y())

def mouseMoveEvent(self, event: QMouseEvent) -> None:
super().mouseMoveEvent(event)
- self.scrollToPos(event.y())

+ self.scrollToPos(qtlib.qMouseEventPosition(event).y())

class BlockMatch(BlockList):

"""
diff --git a/tortoisehg/hgqt/qtlib.py b/tortoisehg/hgqt/qtlib.py
--- a/tortoisehg/hgqt/qtlib.py
+++ b/tortoisehg/hgqt/qtlib.py

@@ -1679,3 +1679,11 @@ def qDropEventPosition(event: QDropEvent

return event.position().toPoint() # pytype: disable=attribute-error
else:
return event.pos() # pytype: disable=attribute-error
+
+

+def qMouseEventPosition(event: QMouseEvent) -> QPoint:
+ """Retrieve QPoint position for the mouse event"""

+ if QT_VERSION >= 0x060000:

+ return event.position().toPoint() # pytype: disable=attribute-error
+ else:
+ return event.pos() # pytype: disable=attribute-error

[Antonio Muci]

That's right, thanks! Sent another version.

[Yuya Nishihara]

On Thu, 20 Mar 2025 15:33:46 +0100, 'Antonio Muci' via TortoiseHg
Developers wrote:
> # HG changeset patch
> # User Antonio Muci <a....@inwind.it>
> # Date 1742481008 -3600
> # Thu Mar 20 15:30:08 2025 +0100
> # Branch stable
> # Node ID 07de97de21ffda32154867da7274ae1b80e9dd9b
> # Parent cde4a42b1a2c6cc3f0ebccdb05df9e4a86986715
> blockmatcher: fix crash on Qt6 when clicking on the left minimap

Queued this, thanks.

> diff --git a/tortoisehg/hgqt/qtlib.py b/tortoisehg/hgqt/qtlib.py
> --- a/tortoisehg/hgqt/qtlib.py
> +++ b/tortoisehg/hgqt/qtlib.py
> @@ -1679,3 +1679,11 @@ def qDropEventPosition(event: QDropEvent
> return event.position().toPoint() # pytype:
> disable=attribute-error else:
> return event.pos() # pytype: disable=attribute-error
> +
> +
> +def qMouseEventPosition(event: QMouseEvent) -> QPoint:

I've added QMouseEvent to the import list. (It wouldn't matter at run
time, though.)