GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-128388: pyrepl on Windows: add meta and ctrl+arrow keybindings by paulie4 · Pull Request #128389 · python/cpython Conversation
|
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
6606f63 to
283e7c3
Compare
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Yes please, and also sign the CLA. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@hugovk, UPDATE: it looks like these are the instructions for which file I should create: https://devguide.python.org/core-developers/committing/#how-to-add-a-news-entry. I think I tried signing the CLA, but the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Yes, that's right, the Lib dir is the stdlib. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…8.8UdMz_.rst` Co-authored-by: Hugo van Kemenade <[email protected]>
|
@hugovk, thank you for that suggested doc change, which I approved. Is there anything else I need to do, or do we now just need to wait for someone else (e.g. @pablogsal, @lysnikolaou, or @ambv) to review the code changes? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for the updates. Yes, someone with Windows will need to review, hopefully it shouldn't be too long :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I left some comments for improvement of the code, but the PR is good: I tested with Windows 11 and the meta keybindings work now
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| return Event( | ||
| evt="key", data=code, raw=rec.Event.KeyEvent.uChar.UnicodeChar | ||
| ) | ||
| if code in ("left", "right") and (ctrlstate := key_event.dwControlKeyState) and ctrlstate & CTRL_OR_ALT_ACTIVE: |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This makes Alt+← and Ctrl-Alt+← work just as Ctrl+←. Is that how it should be?
On Unix for me Alt+← gives me capital C (not sure why, does not really make sense to me) and Ctrl-Alt+← triggers my window manager.
In the keymap I also don't see Alt-left with a special meaning:
Lines 146 to 161 in f157485
| (r"\<up>", "up"), | |
| (r"\<down>", "down"), | |
| (r"\<left>", "left"), | |
| (r"\C-\<left>", "backward-word"), | |
| (r"\<right>", "right"), | |
| (r"\C-\<right>", "forward-word"), | |
| (r"\<delete>", "delete"), | |
| (r"\x1b[3~", "delete"), | |
| (r"\<backspace>", "backspace"), | |
| (r"\M-\<backspace>", "backward-kill-word"), | |
| (r"\<end>", "end-of-line"), # was 'end' | |
| (r"\<home>", "beginning-of-line"), # was 'home' | |
| (r"\<f1>", "help"), | |
| (r"\<f2>", "show-history"), | |
| (r"\<f3>", "paste-mode"), | |
| (r"\EOF", "end"), # the entries in the terminfo database for xterms |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Exactly, I didn't see anything existing that was useful for Alt+←, so I figured might as well make it useful. I can remove it if you prefer only Ctrl+← being useful. Also, it would definitely violate separation of concerns if this Lib/_pyrepl/windows_console.py file were to try to read the keymappings in Lib/_pyrepl/reader.py to check if r"\M-\<left>" existed, and I don't expect that to be added anytime soon.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I would prefer all implementations (unix, windows, etc.) to have the same implementation. Please revert the changes to ALT. There might be good use for the ALT modifier, but that would be for another PR.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
That brings up a good question, why don't we add (r"\M-\<left>", "backward-word") and (r"\M-\<right>", "forward-word") to the default_keymap, so that way all implementations will have something useful for Alt+←?
Also, I just tested that, and it made me realize that the Lib\_pyrepl\windows_console.py code also needs to # queue the key, return the meta command in the special key == "\x00" section (see 46b22d1 for the fix and removal of treating Alt+← as Ctrl+←).
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I guess adding (r"\M-\<left>", "backward-word") and (r"\M-\<right>", "forward-word") to the default_keymap can be part of a separate GH Issue+PR...
Are you ok with the changes I made so we can resolve this conversation?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: Pieter Eendebak <[email protected]>
There was an error while loading. Please reload this page.
| self.event_queue.insert(0, Event(evt="key", data=key, raw=key)) | ||
| return Event(evt="key", data="\033") # keymap.py uses this for meta |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't know the answer. But why pass the ctrl modifier as a key f'ctrl {key}`, and the alt modifier using a special event?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it took me a while to figure out what was going on in Lib\_pyrepl\keymap.py, cause the code is not very readable, but that's what it expects. If we wanted Ctrl and Alt to be treated in a similar way, we'd probably have to make some major edits to Lib\_pyrepl\keymap.py and Lib\_pyrepl\unix_eventqueue.py.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Co-authored-by: Pieter Eendebak <[email protected]>
| CLEAR = "\x1b[H\x1b[J" | ||
|
|
||
| # State of control keys: https://learn.microsoft.com/en-us/windows/console/key-event-record-str | ||
| ALT_ACTIVE = 0x01 | 0x02 |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about including the right ALT in ALT_ACTIVE. On Linux, ALT+Backspace removes a whole word, whereas AltGr+Backspace removes a single character.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Lib/_pyrepl/keymap.py doesn't distinguish between right and left (it just has C- and M-), so I'm not sure that was a conscious decision that was made for the Linux code. Maybe that's actually a bug in the _pyrepl/unix_* code?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Lib/_pyrepl/keymap.py doesn't distinguish between right and left
Python 3.14 on Linux behaves differently for left Alt and right AltGr:
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
On my Linux, the English keyboard layout has Right Alt which generally works like Left Alt (in a terminal).
The French or Czech layout has AltGr instead, which is apparently not Meta enough, so AltGr+Backspace works like Backspace (in both pyrepl and bash).
I tried on some Windows consoles:
| LAlt+Backspace | RAlt+Backspace | AltGr+Backspace | |
|---|---|---|---|
| cmd | Delete single character | Nothing | Nothing |
| powershell | Nothing | Nothing | Nothing |
Doesn't look useful, so it's probably best to have Linux-like behaviour here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what you are saying. Are you saying because the existing functionality isn't useful, we should continue to make the right Alt key not be useful in _pyrepl?
Either way, if this is about AltGr, I think we have to really understand how it works in Windows... I don't have an AltGr key, so I can't test it, but https://en.wikipedia.org/wiki/AltGr_key says, "Windows interprets Ctrl+Alt as AltGr," and since the _KEY_EVENT_RECORD doc doesn't show a dwControlKeyState value for AltGr, doesn't that mean that RIGHT_ALT_PRESSED is not the same thing as AltGr and that the Lib/_pyrepl/windows_console.py code would actually have to look for a combination of Ctrl (left/right) and Alt (left/right) (or maybe just lefts?) being pressed to know if AltGr is being pressed, or am I misunderstanding how the _KEY_EVENT_RECORD works?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I didn't know that other keyboards have a right ALT key different than AltGr. In this case, I'm fine with ALT_ACTIVE = 0x01 | 0x02.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what you are saying.
Sorry for being unclear!
I meant that the behaviour of Windows terminals I tried (cmd & powershell) is not useful, so Python should not emulate that.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't have an AltGr key, so I can't test it
It's a feature of the keyboard layout, not the physical keyboard. To test it you can add, for example, French "AZERTY" in system settings.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM. It works as expected: CTRL+Left skips a whole word and ALT+Backspace deletes a word.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| CLEAR = "\x1b[H\x1b[J" | ||
|
|
||
| # State of control keys: https://learn.microsoft.com/en-us/windows/console/key-event-record-str | ||
| ALT_ACTIVE = 0x01 | 0x02 |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I didn't know that other keyboards have a right ALT key different than AltGr. In this case, I'm fine with ALT_ACTIVE = 0x01 | 0x02.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…gs (pythonGH-128389) Fix `Lib/_pyrepl/windows_console.py` to support more keybindings, like the `Ctrl`+`←` and `Ctrl`+`→` word-skipping keybindings and those with meta (i.e. Alt), e.g. to `kill-word` or `backward-kill-word`. Specifics: if Ctrl is pressed, emit "ctrl left" and "ctrl right" instead of just "left" or "right," and if Meta/Alt is pressed, emit the special key code for meta before emitting the other key that was pressed. (cherry picked from commit 688f3a0) Co-authored-by: Paulie Peña <[email protected]> Co-authored-by: Hugo van Kemenade <[email protected]> Co-authored-by: Pieter Eendebak <[email protected]>
|
GH-130500 is a backport of this pull request to the 3.13 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…ngs (GH-128389) (GH-130500) gh-128388: pyrepl on Windows: add meta and ctrl+arrow keybindings (GH-128389) Fix `Lib/_pyrepl/windows_console.py` to support more keybindings, like the `Ctrl`+`←` and `Ctrl`+`→` word-skipping keybindings and those with meta (i.e. Alt), e.g. to `kill-word` or `backward-kill-word`. Specifics: if Ctrl is pressed, emit "ctrl left" and "ctrl right" instead of just "left" or "right," and if Meta/Alt is pressed, emit the special key code for meta before emitting the other key that was pressed. (cherry picked from commit 688f3a0) Co-authored-by: Paulie Peña <[email protected]> Co-authored-by: Hugo van Kemenade <[email protected]> Co-authored-by: Pieter Eendebak <[email protected]>
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.