Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Mac qt ctrl#21142
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Mac qt ctrl#21142
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The alias SIGCLD for SIGCHLD does not exist on OSX
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This works for me locally, it also looks like it is being hit in CI now (there are no skips in test_backend_qt on macos-latest). Just one minor comment for readability.
| ( | ||
| "Key_A", | ||
| ["ControlModifier"], | ||
| ("ctrl+a" if sys.platform != "darwin" else "cmd+a"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I find the previous version a little easier to read with more on a single line. Perhaps you could leave the parametrize section alone and put the switch inside the function?
ifsys.platform=="darwin":qt_mods.replace("ctrl","cmd")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fair, that did not occur to me to try! I was going going through the failing tests one at a time and putting the fix in.
On the other hand, changing the parameters makes it clear from looking at the test the expected behavior is which might be clearer in the long run?
| '\N{LATIN CAPITAL LETTER A WITH ACUTE}'), | ||
| ('Key_Aacute', [], | ||
| '\N{LATIN SMALL LETTER A WITH ACUTE}'), | ||
| ('Key_Control', ['AltModifier'], 'alt+control'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm not sure why this is "control" rather than "ctrl" here, but it appears to work either way for me locally, and this would get you an easier string replacement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think the issue is that in some cases it is 'ctrl' because it is a modifier, but here it is 'control' because it is the key. If this distinction is a good idea or not is a different issue, but this fails on linux if we change control -> ctrl here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There's something wrong with 'ctrl' vs. 'control'.
skip pyside2 on GHA because it is segfaulting with openGL issues
Due to some re-(re)-mapping of the modifier keys by the OSX keyboard,then by Qt, and then by us there is an inconsistency between theplatforms.This change to the tests encodes what the Qt on OSX currently does (andto our understand have always done).
…142-on-v3.5.xBackport PR#21142 on branch v3.5.x (Mac qt ctrl)
PR Summary
closes#21110
This fixes the QT tests to pass on OSX and tries to enable running them on GHA.
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).