Hi All,
this morning I've looked at some platform even t code to address an issue with scrolling that Chris Muller was bitten by. In reading the code I've tried to make the code comprehensible to others who may come to read it. In particular I changed this code in platforms/iOS/vm/OSX/sqSqueakOSXApplication+events.m
evt.charCode = keyCode; evt.utf32Code = 0; evt.reserved1 = 0; evt.modifiers = (CtrlKeyBit | OptionKeyBit | CommandKeyBit | ShiftKeyBit); evt.windowIndex = windowIndex; [self pushEventToQueue:(sqInputEvent *) &evt];
to read evt.charCode = keyCode; evt.utf32Code = 0; evt.reserved1 = 0; /* Set every meta bit to distinguish the fake event from a real right/left * arrow. */ evt.modifiers = (CtrlKeyBit | OptionKeyBit | CommandKeyBit | ShiftKeyBit); evt.windowIndex = windowIndex; [self pushEventToQueue:(sqInputEvent *) &evt];
After all, what the code does is obvious, but /why/ the code does what it does is not at all obvious.
In reading similar code on the other platforms I see things like this, an example from platforms/win32/vm/sqWin32Window.c:
evt->modifiers = CtrlKeyBit; #ifdef PharoVM evt->utf32Code = evt->charCode; #else evt->utf32Code = 0; #endif evt->reserved1 = 0;
Again, *why* the code sets utf32Code but not on Squeak, Newspeak, etc, is not at all obvious and it would be really helpful if it were commented so that the decision as to whether the ifdef might be eliminated and the code always set6 (surely preferable) would be made easier.
The lack of such comments makes managing this code very hard for those of us that are not platform experts but must maintain the code across all platforms because that's our lot in life. The absence of such comments, when one is struggling to figure out what the hell is going on, feels like passive aggression ;-) So if you've made some m modification to the platform code in the past few years and you haven't commented it to explain the *why*, please feel free to comment the code and commit or submit a pull request; it will be warmly appreciated.
Thank you!
_,,,^..^,,,_ best, Eliot
Ok, so in reviewing GitHub, this change comes from https://github.com/pharo-project/pharo-vm/commit/e80301237d8f23d0da0db80fe1e...
"Fake mouse wheel events with all modifier keys pressed (not just ctrl)"
What made it unclear was I hand merged the Pharo & Pyonkee changes in early 2015, thus losing the historical Git information found in the pharo git repository. In this case I'll suspect there is changes to the event handling in Pharo which perhaps are not in Squeak Smalltalk base?
JMM - integrate Pyonkee & Pharo changes to cocoa os-x and iPhone code base git-svn-id: http://squeakvm.org/svn/squeak/trunk@3324 fa1542d4-bde8-0310-ad64-8ed1123d492a Former-commit-id: e11f52c96c104466f3636f9cfb8f1d8bc4219b56 Commit:346a75a0cbf554bfe5aed0281283fd19f8a38fef [346a75a0c] Parents:839962918e Author:John McIntosh johnmci@smalltalkconsulting.com Date:May 7, 2015 at 11:23:28 AM PDT
.... John M. McIntosh. Corporate Smalltalk Consulting Ltd https://www.linkedin.com/in/smalltalk
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Wednesday, November 7, 2018 7:32 AM, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi All,
this morning I've looked at some platform even t code to address an issue with scrolling that Chris Muller was bitten by. In reading the code I've tried to make the code comprehensible to others who may come to read it. In particular I changed this code in platforms/iOS/vm/OSX/sqSqueakOSXApplication+events.m evt.charCode = keyCode; evt.utf32Code = 0; evt.reserved1 = 0; evt.modifiers = (CtrlKeyBit | OptionKeyBit | CommandKeyBit | ShiftKeyBit); evt.windowIndex = windowIndex; [self pushEventToQueue:(sqInputEvent *) &evt];to read evt.charCode = keyCode; evt.utf32Code = 0; evt.reserved1 = 0; /* Set every meta bit to distinguish the fake event from a real right/left * arrow. */ evt.modifiers = (CtrlKeyBit | OptionKeyBit | CommandKeyBit | ShiftKeyBit); evt.windowIndex = windowIndex; [self pushEventToQueue:(sqInputEvent *) &evt];
After all, what the code does is obvious, but /why/ the code does what it does is not at all obvious.
In reading similar code on the other platforms I see things like this, an example from platforms/win32/vm/sqWin32Window.c:
evt->modifiers = CtrlKeyBit;#ifdef PharoVM evt->utf32Code = evt->charCode; #else evt->utf32Code = 0; #endif evt->reserved1 = 0;
Again, *why* the code sets utf32Code but not on Squeak, Newspeak, etc, is not at all obvious and it would be really helpful if it were commented so that the decision as to whether the ifdef might be eliminated and the code always set6 (surely preferable) would be made easier.
The lack of such comments makes managing this code very hard for those of us that are not platform experts but must maintain the code across all platforms because that's our lot in life. The absence of such comments, when one is struggling to figure out what the hell is going on, feels like passive aggression ;-) So if you've made some m modification to the platform code in the past few years and you haven't commented it to explain the *why*, please feel free to comment the code and commit or submit a pull request; it will be warmly appreciated.
Thank you!
_,,,^..^,,,_ best, Eliot
johnmci wrote
"Fake mouse wheel events with all modifier keys pressed (not just ctrl)"
I thought that looked familiar. I needed that change IIRC to enable horizontal scrolling in Pharo because there was a shortcut conflict. However, I got bogged down when I went to make the change in Windows and then got distracted. The reason I chose a more complex hack instead of a clean fix is because the solution seemed backward compatible so it didn't break Squeak et al.
----- Cheers, Sean -- Sent from: http://forum.world.st/Squeak-VM-f104410.html
Hi John, Hi Sean,
On Wed, Nov 7, 2018 at 2:16 PM Sean P. DeNigris sean@clipperadams.com wrote:
johnmci wrote
"Fake mouse wheel events with all modifier keys pressed (not just ctrl)"
I thought that looked familiar. I needed that change IIRC to enable horizontal scrolling in Pharo because there was a shortcut conflict. However, I got bogged down when I went to make the change in Windows and then got distracted. The reason I chose a more complex hack instead of a clean fix is because the solution seemed backward compatible so it didn't break Squeak et al.
I'm sure I speak for everyone in being very grateful for the code! My point is not to call either of you out on this specific issue & commit. It is rather to ask people to put more effort into commenting the platform code. It belongs to all of us but is not Smalltalk, is often platform-specific, and crosses the boundary between platform and Smalltalk, so it is complex and mysterious, and more commentary is very helpful.
Cheers,
Sean
_,,,^..^,,,_ best, Eliot
On 2018-11-08, at 11:07 AM, Eliot Miranda eliot.miranda@gmail.com wrote:
I'm sure I speak for everyone in being very grateful for the code! My point is not to call either of you out on this specific issue & commit. It is rather to ask people to put more effort into commenting the platform code. It belongs to all of us but is not Smalltalk, is often platform-specific, and crosses the boundary between platform and Smalltalk, so it is complex and mysterious, and more commentary is very helpful.
Exactly. Comments need to explain why, why not, what is done, what is still needed someday, obscure points not necessarily obvious to a non-platform expert and so on. If possible point to the doc that explains it too. Yes, the code says what actually happens but anyone that thinks that 'actual' == 'needed' is likely to be very disappointed, very often.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Useful random insult:- Not much to show for four billion years of evolution.
vm-dev@lists.squeakfoundation.org
-
Eliot Miranda -
John M McIntosh -
Sean P. DeNigris -
tim Rowledge