[OpenSmalltalk/opensmalltalk-vm] Elided code that caused ctrl keyboard shortcuts to fail on 32b images (#400)

 

This patch solves the problem discussed in #396

---

You can view, comment on, or merge this pull request online at:

  https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400

Commit Summary

• elided code that caused ctrl keyboard shortcuts to fail on 32b images

File Changes

• M platforms/unix/vm-display-X11/sqUnixX11.c (8)

Patch Links:

• https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400.patch
• https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEWZQUD26Y5HXGGPTR7LPZUJZNA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GYOBKIQ", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEWZQUD26Y5HXGGPTR7LPZUJZNA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GYOBKIQ", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

I think we really should wait with this until Eliot's approval. He put the code in there not too long ago, and I'm sure he had good reason to…

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW6AYUZE45SQMX5LRN3PZUPC3A5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXIMMWA#issuecomment-500221528", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW6AYUZE45SQMX5LRN3PZUPC3A5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXIMMWA#issuecomment-500221528", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

I agree with Tobias, I suggest that we analyze the failure further, specifically what happens to modifiers state before reverting those lines. I commented Issue #396 with a bit more information about my yet incomplete understanding of the problem.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEWYH4ZGKEDYMJKQ3RZTP2AKT5A5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXOSATQ#issuecomment-501030990", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEWYH4ZGKEDYMJKQ3RZTP2AKT5A5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXOSATQ#issuecomment-501030990", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 
On Sun, Jun 9, 2019 at 8:33 AM Tobias Pape <[hidden email]> wrote:

> I think we really should wait with this until Eliot's approval. He put the
> code in there not too long ago, and I'm sure he had good reason to…
>

IIRC, I just rubber stamped someone else's contribution.

_,,,^..^,,,_
best, Eliot

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW7NKSHV5PXRXO4YSX3P2BHYLA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXO7USY#issuecomment-501086795", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW7NKSHV5PXRXO4YSX3P2BHYLA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXO7USY#issuecomment-501086795", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

I think this is the relevant issue: feenkcom/gtoolkit#9 (comment)
See John Brant's comments from 14 December.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW6W3O33XDDVLKN4AWTP2D65PA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXQQ3QA#issuecomment-501288384", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW6W3O33XDDVLKN4AWTP2D65PA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXQQ3QA#issuecomment-501288384", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

 

Good find @akgrant43. Paraphrasing feenkcom/gtoolkit#9...

Girba: A piece of information about this bug. There is an inconsistency in Mac and Linux VMs when it comes to keyboard events:
OSX generates keyDown: events for keys like shift , ctrl or meta when I press the corresponding keys, while
Linux VM generates keyDown: events for ctrl when we release the key
Our issue is at a lower level then Morphic.
@syrel used logging statements in HandMorph>>#generateKeyboardEvent:. It appears that it is not called when we press a modifier key on Linux.

syrel: Indeed, the keyboard events for shift are sent but the question is when they are sent. We state that they are not sent when we press the shift button, but instead when we release it. This is a very important and crucial difference that does not let us to have simple and clear shortcut handling in Bloc on linux. In Bloc by design we assume that whenever a key is pressed - we get a keydown event (hence it has down in it), right after that for printable characters we get keystroke (or input) event that gets resent every X seconds depending on OS settings and finally when user releases her finger we get keyup event. Unfortunately this does not happen for keys like shift, alt and ctrl while perfectly works for regular keys like A, M, Z.
@akgrant43 I have no idea why it works for you but doesn’t work for multiple other people. We are debugging this for the latest Pharo7 64bit with latest VM on Linux Mint 64bit.

akgrant43: I'm not seeing this behaviour (Ubuntu 16.04).
Adding logging to:
Morph>>handleKeyDown:
Morph>>handleKeyUp:
Shows them being called at the expected times, i.e. as the ctrl key is being pressed and released.
Linux Mint, if I remember correctly, is typically based on a fairly old version of Ubuntu (not that mine is recent :-)). Maybe there's a change in the kernel or graphics stack?

j-brant: I have gotten the keyboard shortcuts to work under my Linux Mint installation. There were a few issues. First, the KeyboardKey class was using the wrong key table for Unix. Second, the right Unix key table was missing the right control & alt keys. These were addressed in https://pharo.manuscript.com/f/cases/22769 and have been integrated into the latest Pharo build.
.
The other issues were VM issues. When you pressed and then released either the control or shift keys, the correct key would be recorded as pressed, but character code $? would be reported when the key was released. This causes the editor to believe the control key was still pressed even after it was released since it never got the key up for the control. The other VM issue was that A-Z keys being pressed when the control was pressed was being reported as character code 1-26 instead of the letters a-z. On the other platforms, they are reported as letters with the control key pressed. On Linux, they were reported with the control key down, but with character codes 1-26. To fix the VM issues, I added some code to the x2sqKeyPlain method in sqUnixX11.c file before the return (j-brant/opensmalltalk-vm@37e3abf):

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW3FLPU5YOBWBYNMGF3P2EB7XA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXQTWHI#issuecomment-501299997", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW3FLPU5YOBWBYNMGF3P2EB7XA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXQTWHI#issuecomment-501299997", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

Closed #400.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW4LH5VQUQHHLHTJNJDP2HGFRA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOR6N4R2Y#event-2409351403", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW4LH5VQUQHHLHTJNJDP2HGFRA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOR6N4R2Y#event-2409351403", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

John Brant reported that this patch breaks ctrl-a handling in 64b image. So this patch will not solve the issue for all Squeak and Pharo images. I will withdraw the patch. Further discussions can continue in #396

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW6JKHBVKHTVHLUAD23P2HGFRA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXSP4PI#issuecomment-501546557", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/400?email_source=notifications\u0026email_token=AIJPEW6JKHBVKHTVHLUAD23P2HGFRA5CNFSM4HWJSRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXSP4PI#issuecomment-501546557", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>