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

fix use of ESC in settings window #516

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

Conversation

sfo
Copy link
Contributor

@sfo sfo commented Jul 25, 2024

When pressing ESC in settings window the application crashed due to missing attribute.

@jooste
Copy link
Member

jooste commented Jul 25, 2024

This is an api difference between qt5 and qt6. We're considering dropping support for qt5, which would make this easier, because now your change fixes qt6, but breaks qt5 :)

@sfo
Copy link
Contributor Author

sfo commented Jul 29, 2024

Thanks for the hint. I had this in mind, as well, and checked the rest of the code:

stanley@gallifrey:~/Development/bluesky|master ⇒  grep "Qt.Key_" -R .  
./bluesky/ui/qtgl/infowindow.py:    #     if event.key() == Qt.Key_Escape:
./bluesky/ui/qtgl/settingswindow.py:        if event.key() == Qt.Key_Escape:
grep: ./utils/Doc generation/bluesky.wiki: No such file or directory
stanley@gallifrey:~/Development/bluesky|master ⇒  grep "Qt\.Key\." -R .
./bluesky/ui/qtgl/console.py:        if event.key() == Qt.Key.Key_Enter or event.key() == Qt.Key.Key_Return:
./bluesky/ui/qtgl/console.py:        if event.key() >= Qt.Key.Key_Space and event.key() <= Qt.Key.Key_AsciiTilde:
./bluesky/ui/qtgl/console.py:        elif event.key() == Qt.Key.Key_Backspace:
./bluesky/ui/qtgl/console.py:        elif event.key() == Qt.Key.Key_Tab:
./bluesky/ui/qtgl/console.py:            if event.key() == Qt.Key.Key_Up:
./bluesky/ui/qtgl/console.py:            elif event.key() == Qt.Key.Key_Down:
./bluesky/ui/qtgl/console.py:            elif event.key() == Qt.Key.Key_Left:
./bluesky/ui/qtgl/console.py:            elif event.key() == Qt.Key.Key_Right:
./bluesky/ui/qtgl/mainwindow.py:                and event.key() in [Qt.Key.Key_Up, Qt.Key.Key_Down, Qt.Key.Key_Left, Qt.Key.Key_Right]:
./bluesky/ui/qtgl/mainwindow.py:            if event.key() == Qt.Key.Key_Up:
./bluesky/ui/qtgl/mainwindow.py:            elif event.key() == Qt.Key.Key_Down:
./bluesky/ui/qtgl/mainwindow.py:            elif event.key() == Qt.Key.Key_Left:
./bluesky/ui/qtgl/mainwindow.py:            elif event.key() == Qt.Key.Key_Right:
./bluesky/ui/qtgl/mainwindow.py:        elif event.key() == Qt.Key.Key_Escape:
./bluesky/ui/qtgl/mainwindow.py:        elif event.key() == Qt.Key.Key_F11:  # F11 = Toggle Full Screen mode
./extra/textclient/textclient.py:        if event.key() == Qt.Key.Key_Enter or event.key() == Qt.Key.Key_Return:
grep: ./utils/Doc generation/bluesky.wiki: No such file or directory

The results show that the settings window was the only one still using the Qt5 API. In all other places, the Qt6 API is used, even in mainwinow:

elif event.key() == Qt.Key.Key_Escape:

Therefore, I thought it's just fine. Maybe, I am still missing something? Then I could of course adapt the PR to support Qt5 and 6, if required.

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.

2 participants