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

Keyboard handling rework #292

Merged

Conversation

benoit-pierre
Copy link
Member

  • cleanup/simplify handling of keyboard events
  • cleanup keyboard test code
  • make keymap handling a little more robust (particularly: add missing entries, so they show up and can be configured in the GUI)
  • add support for using the function keys F1-F12
  • add support for configuring the list of additional keys to be suppressed (keys that are not mapped to steno keys): add an extra 'no-op' entry to the keymap
  • add support for configuring the arpeggiate key: add an extra 'arpeggiate' entry to the keymap

Note:

  • I tested the Linux version, as well as the Windows version with Wine
  • I think it would be pretty easy to add support for configuring a global shortcut for toggling Plover on/off with the oslayer/keyboardcontrol code, since we can suppress and listen to all key events

@KarlHegbloom
Copy link
Contributor

Traceback (most recent call last):
File "/usr/lib/python2.7/dist-packages/plover/gui/config.py", line 332, in show_add_translation
plover.gui.add_translation.Show(self, self.engine, self.config)
File "/usr/lib/python2.7/dist-packages/plover/gui/add_translation.py", line 210, in Show
util.SetTopApp()
File "/usr/lib/python2.7/dist-packages/plover/gui/util.py", line 53, in SetTopApp
call(['wmctrl', '-a', TITLE])
NameError: global name 'TITLE' is not defined

@benoit-pierre
Copy link
Member Author

It's an unrelated bug: #222

@morinted
Copy link
Member

Any insight into #222 that you can give, Ben? He's reporting that error. Is there a fix we can make or should that PR be pulled as-is?

@KarlHegbloom
Copy link
Contributor

Ok. I agree, since it's coming from a part of the code not touched directly by your patch; but naively wondering if changes to method call signatures in your patches could have caused it. Now I see that it's not the case.

Other than that, it works as expected. I can use standalone plover in Emacs without it blonking on the function key presses. It also works fine with ibus-plover as before. The NKRO keyboard configuration dialog works fine and shows the same configuration as I'd set up previously with the addition of the new rows. I think this change set is good to go; I'm running Ubuntu vivid.

@benoit-pierre
Copy link
Member Author

@morinted: the changes in #222 look OK and work as expected. So yeah, it can probably be merged with one tweak: cleanup the whitespace related changes in SetTopApp (trailling space line 54 and bad indentation line 57).

@morinted
Copy link
Member

Wonderful, I'll get to that tonight. Then I'll start looking at all your changes. No guarantees on getting it done before Halloween ;)

@KarlHegbloom
Copy link
Contributor

I also noticed that in one file at least there are DOS line endings for
part of the file. The Python manual recommends putting a coding line at the
top of each file so that editors when they open it will use the right
coding system. That should probably be added to all of the files and then
something right on them to ensure that they all have the same coding system
and line ending convention. I think they should be utf-8 Unix.

On Thu, Oct 29, 2015, 13:36 Ted Morin notifications@github.com wrote:

Wonderful, I'll get to that tonight. Then I'll start looking at all your
changes. No guarantees on getting it done before Halloween ;)


Reply to this email directly or view it on GitHub
#292 (comment)
.

@morinted
Copy link
Member

I agree, ideally I'd like to be able to do have the code base be PEP8'd, but I'm not in favor of doing it for the sake of doing it. As I touch files I'll add the line ending and code style preferences. That being said, our code is seriously lacking adaptors for most of the C-libraries.

@KarlHegbloom
Copy link
Contributor

Please explain what you mean by "our code is seriously lacking adaptors for
most of the C-libraries". Why would Plover need adaptors for C?

Perhaps the line ending change should be done all at once, in one big
commit on master, so everyone can easily rebase, or just run the same
line-ending normalizer on their own copy? Because of the way that git works
wrt SHA1, it wouldn't cause too many conflicts, since if we each run the
same deterministic transformation on our own separate copies of a file, the
SHA1 will match. Putting all of the line ending changes into one change set
isolates them from diffs in other later change sets. Anyone working on a
branch will need to either rebase or run the same normalizer though, or
their patch will put the old wrong line endings back.

On Fri, Oct 30, 2015, 09:35 Ted Morin notifications@github.com wrote:

I agree, ideally I'd like to be able to do have the code base be PEP8'd,
but I'm not in favor of doing it for the sake of doing it. As I touch files
I'll add the line ending and code style preferences. That being said, our
code is seriously lacking adaptors for most of the C-libraries.


Reply to this email directly or view it on GitHub
#292 (comment)
.

@morinted
Copy link
Member

https://www.youtube.com/watch?v=wf-BqAjZb8M

He uses an adaptor in this video to make code more "Pythonic".

For the line endings, I'll probably just use a .gitattributes. I'd say right now it's not high on my priority list compared to all the other pending changes and bugs.

We do not care about the keysym.
@balshetzer
Copy link
Member

That video is way too long. Can you point to these c adapters another way?
On Oct 30, 2015 1:32 PM, "Ted Morin" notifications@github.com wrote:

https://www.youtube.com/watch?v=wf-BqAjZb8M

He uses an adaptor in this video to make code more "Pythonic".

For the line endings, I'll probably just use a .gitattributes. I'd say
right now it's not high on my priority list compared to all the other
pending changes and bugs.


Reply to this email directly or view it on GitHub
#292 (comment)
.

@morinted
Copy link
Member

I'm having trouble finding a brief explanation, but it's just a simple wrapping class to provide a more attractive interface to some kind of object; it's not C-exclusive.

The point is to abstract away C/Java/other interfaces into something that is more native-feeling to Python. Instead of using libraryname.getObjectAtIndex(i), you could wrap around it so that you can use libraryname[i]. Here's a long post with code examples.

I don't have lots of experience writing "good" Python code, and I'm really trying to dig into it while working with Plover. If you know of any good resources on Python best practices, I'd be interested in those, too.

@KarlHegbloom
Copy link
Contributor

I remember learning about how Python uses "duck typing"... if it looks like
a duck, walks like a duck, and quacks like a duck, it's a duck.

On Sat, Oct 31, 2015, 09:04 Ted Morin notifications@github.com wrote:

I'm having trouble finding a brief explanation, but it's just a simple
wrapping class to provide a more attractive interface to some kind of
object; it's not C-exclusive.

The point is to abstract away C/Java/other interfaces into something that
is more native-feeling to Python. Instead of using
libraryname.getObjectAtIndex(i), you could wrap around it so that you can
use libraryname[i]. Here's a long post with code examples.
http://ginstrom.com/scribbles/2009/03/27/the-adapter-pattern-in-python/

I don't have lots of experience writing "good" Python code, and I'm really
trying to dig into it while working with Plover. If you know of any good
resources on Python best practices, I'd be interested in those, too.


Reply to this email directly or view it on GitHub
#292 (comment)
.

@benoit-pierre
Copy link
Member Author

So @KarlHegbloom, about your problem reported here benoit-pierre/ibus-plover#2 (comment):

  • your testing on Linux right?
  • does it happen every time?
  • do you use the Plover button, or a stroke to disable Plover?
  • after disabling it, can you re-enable Plover?
  • any trace?

Suppress all supported keys.
@KarlHegbloom
Copy link
Contributor

I finally got around to testing that. It's working fine. I think that the
problem happened when Plover crashed or I killed it while the keys were
suppessed... but that doesn't make sense either, since the keyboard grab
goes away when the program exits. I guess "never mind" this one, unless it
happens again.

It's working wonderfully right now, using the function and number keys. No
"help" when I use F1 as S-, and when I turn Plover off while it's still
running, they are functio and number keys again, though the paper-tape
trail keeps seeing them as steno strokes. When it's off, I can type
normally, but again, the suggestions and trail see the use of the e r u i
keys as steno, while I qwerty type. I think those should be made to stop
when Plover is "off" (red P).

On Mon, Nov 2, 2015 at 4:53 AM, Benoit Pierre notifications@github.com
wrote:

So @KarlHegbloom https://github.com/KarlHegbloom, about your problem
reported here benoit-pierre/ibus-plover#2 (comment)
benoit-pierre/ibus-plover#2 (comment)
:

  • your testing on Linux right?
  • does it happen every time?
  • do you use the Plover button, or a stroke to disable Plover?
  • after disabling it, can you re-enable Plover?
  • any trace?


Reply to this email directly or view it on GitHub
#292 (comment)
.

Karl.Hegbloom@gmail.com
http://karlhegbloom.blogspot.com

@morinted
Copy link
Member

morinted commented Nov 5, 2015

Yeah Plover basically doesn't stop running. No idea why it was designed
like that. Insight?
On Nov 4, 2015 7:28 PM, "Karl Hegbloom" notifications@github.com wrote:

I finally got around to testing that. It's working fine. I think that the
problem happened when Plover crashed or I killed it while the keys were
suppessed... but that doesn't make sense either, since the keyboard grab
goes away when the program exits. I guess "never mind" this one, unless it
happens again.

It's working wonderfully right now, using the function and number keys. No
"help" when I use F1 as S-, and when I turn Plover off while it's still
running, they are functio and number keys again, though the paper-tape
trail keeps seeing them as steno strokes. When it's off, I can type
normally, but again, the suggestions and trail see the use of the e r u i
keys as steno, while I qwerty type. I think those should be made to stop
when Plover is "off" (red P).

On Mon, Nov 2, 2015 at 4:53 AM, Benoit Pierre notifications@github.com
wrote:

So @KarlHegbloom https://github.com/KarlHegbloom, about your problem
reported here benoit-pierre/ibus-plover#2 (comment)
<
benoit-pierre/ibus-plover#2 (comment)

:

  • your testing on Linux right?
  • does it happen every time?
  • do you use the Plover button, or a stroke to disable Plover?
  • after disabling it, can you re-enable Plover?
  • any trace?


Reply to this email directly or view it on GitHub
<
#292 (comment)

.

Karl.Hegbloom@gmail.com
http://karlhegbloom.blogspot.com


Reply to this email directly or view it on GitHub
#292 (comment)
.

@balshetzer
Copy link
Member

Plover keeps listening so it can process the on or toggle command. The
other stuff is just hooked into the stream and, I guess, isn't very
intelligent about being turned on and off.
On Nov 4, 2015 10:12 PM, "Ted Morin" notifications@github.com wrote:

Yeah Plover basically doesn't stop running. No idea why it was designed
like that. Insight?
On Nov 4, 2015 7:28 PM, "Karl Hegbloom" notifications@github.com wrote:

I finally got around to testing that. It's working fine. I think that the
problem happened when Plover crashed or I killed it while the keys were
suppessed... but that doesn't make sense either, since the keyboard grab
goes away when the program exits. I guess "never mind" this one, unless
it
happens again.

It's working wonderfully right now, using the function and number keys.
No
"help" when I use F1 as S-, and when I turn Plover off while it's still
running, they are functio and number keys again, though the paper-tape
trail keeps seeing them as steno strokes. When it's off, I can type
normally, but again, the suggestions and trail see the use of the e r u i
keys as steno, while I qwerty type. I think those should be made to stop
when Plover is "off" (red P).

On Mon, Nov 2, 2015 at 4:53 AM, Benoit Pierre notifications@github.com
wrote:

So @KarlHegbloom https://github.com/KarlHegbloom, about your problem
reported here benoit-pierre/ibus-plover#2 (comment)
<

benoit-pierre/ibus-plover#2 (comment)

:

  • your testing on Linux right?
  • does it happen every time?
  • do you use the Plover button, or a stroke to disable Plover?
  • after disabling it, can you re-enable Plover?
  • any trace?


Reply to this email directly or view it on GitHub
<

#292 (comment)

.

Karl.Hegbloom@gmail.com
http://karlhegbloom.blogspot.com


Reply to this email directly or view it on GitHub
<
#292 (comment)

.


Reply to this email directly or view it on GitHub
#292 (comment)
.

@KarlHegbloom
Copy link
Contributor

I wish I had more time for it now... after another 6 months or a year, I
should have it; I've clone some repositories in my github that I plan to
study: autokey, plover, ibus-plover, and etc. I wonder if autokey, plover,
and ibus-table have anything in common, for example... How does each do
similar things? Which one seems the best? Can we learn anything from the
others to improve Plover?

On Wed, Nov 4, 2015 at 9:23 PM, Hesky Fisher notifications@github.com
wrote:

Plover keeps listening so it can process the on or toggle command. The
other stuff is just hooked into the stream and, I guess, isn't very
intelligent about being turned on and off.

On Nov 4, 2015 10:12 PM, "Ted Morin" notifications@github.com wrote:

Yeah Plover basically doesn't stop running. No idea why it was designed
like that. Insight?
On Nov 4, 2015 7:28 PM, "Karl Hegbloom" notifications@github.com
wrote:

I finally got around to testing that. It's working fine. I think that
the
problem happened when Plover crashed or I killed it while the keys were
suppessed... but that doesn't make sense either, since the keyboard
grab
goes away when the program exits. I guess "never mind" this one, unless
it
happens again.

It's working wonderfully right now, using the function and number keys.
No
"help" when I use F1 as S-, and when I turn Plover off while it's still
running, they are functio and number keys again, though the paper-tape
trail keeps seeing them as steno strokes. When it's off, I can type
normally, but again, the suggestions and trail see the use of the e r
u i
keys as steno, while I qwerty type. I think those should be made to
stop
when Plover is "off" (red P).

On Mon, Nov 2, 2015 at 4:53 AM, Benoit Pierre <
notifications@github.com>
wrote:

So @KarlHegbloom https://github.com/KarlHegbloom, about your
problem
reported here benoit-pierre/ibus-plover#2 (comment)
<

benoit-pierre/ibus-plover#2 (comment)

:

  • your testing on Linux right?
  • does it happen every time?
  • do you use the Plover button, or a stroke to disable Plover?
  • after disabling it, can you re-enable Plover?
  • any trace?


Reply to this email directly or view it on GitHub
<

#292 (comment)

.

Karl.Hegbloom@gmail.com
http://karlhegbloom.blogspot.com


Reply to this email directly or view it on GitHub
<

#292 (comment)

.


Reply to this email directly or view it on GitHub
<
#292 (comment)

.


Reply to this email directly or view it on GitHub
#292 (comment)
.

Karl.Hegbloom@gmail.com
http://karlhegbloom.blogspot.com

@Achim63
Copy link

Achim63 commented Nov 5, 2015

I think it's a good idea to have Plover running all the time - if you don't use a qwerty-keyboard but a steno machine, you'll want it to be ready to go at all times. And of course Plover needs to listen for the on/off strokes for qwerty users, as Hesky mentioned already.

It might even be possible to let the suggestions work while you use qwerty by picking up the typed words and doing a lookup. For now I'm using my lookup script if I'm too lazy to fingerspell something and reach out to the qwerty-keyboard to type it.

@KarlHegbloom
Copy link
Contributor

Good idea. Can you put that into a todo item in the source or a text file
in the repo please? It's a good weekend project idea for somebody someday.
With a dual keyboard, it makes a lot of sense to flip to qwerty (or dvorak
or colemak) rather than using the traditional steno fingerspelling. And
while you're doing that, it may as well suggest the steno for the thing you
typed, and maybe even suggest a stroke to assign it to, using some
algorithm that operates according to the theory by which briefs are being
defined, choosing a stroke that is not defined to anything.

Perhaps that can be switched on and off also? Or maybe just minimized or
the window knocked down. But when that window is not being displayed,
there's no reason for the process to be running that produces those
suggestions. Also at the same time, there's not really any reason why
Plover needs to be dribbling the keystrokes into the log file when it is
not in steno mode, or running the paper tape display.

On Thu, Nov 5, 2015, 05:14 Achim notifications@github.com wrote:

I think it's a good idea to have Plover running all the time - if you
don't use a qwerty-keyboard but a steno machine, you'll want it to be ready
to go at all times. And of course Plover needs to listen for the on/off
strokes, also for qwerty users, as Hesky mentioned already.

It might even be possible to let the suggestions work while you use qwerty
by picking up the typed words and doing a lookup. For now I'm using my
lookup script if I'm too lazy to fingerspell something and reach out to the
qwerty-keyboard to type it.


Reply to this email directly or view it on GitHub
#292 (comment)
.

@morinted
Copy link
Member

morinted commented Nov 7, 2015

Noticed some conflicting behavior:

If I have "space" in the noop line and turn on arpeggiate, it works and arpeggiates. If I turn off arpeggiate, it successfully still suppresses space. Great.

However, if I add a key to both a steno line and the noop line, that key is just suppressed.

I think I prefer the arpeggiate line behavior -> I should be able to add the whole alphabet to "noop" and still have my set keys work for steno. Does that make sense?

@benoit-pierre
Copy link
Member Author

Well, the keymap code does not do any sanity checks, so behavior with conflicting entries is undefined.

I think having one key in multiple entries should not be allowed.

I think I prefer the arpeggiate line behavior -> I should be able to add the whole alphabet to "noop" and still have my set keys work for steno. Does that make sense?

I don't think it make sense, I mean, why even go through the trouble of adding all those keys to the noop line? Unless of course, we were to support a more compact way to list those keys (like '[a-z]', or '[:alpha:]'.

@morinted
Copy link
Member

morinted commented Nov 7, 2015

Either behavior is all right, as long as it's handled. In my opinion, I'd
rather have it override the noop line, in case someone adds to his keymap
and doesn't see the keys already in the noop line. It would be puzzling as
to why it doesn't work.

Furthermore, we don't report when keys are used twice in the list, as the
configuration is just a simple text box. In the case of the user defining
"q" as "T-" and also including "q" in noop, I would think that the user
intended for "T-" to be "q". That's my intuition, at least.

On Sat, Nov 7, 2015 at 4:10 PM Benoit Pierre notifications@github.com
wrote:

Well, the keymap code does not do any sanity checks, so behavior with
conflicting entries is undefined.

I think having one key in multiple entries should not be allowed.

I think I prefer the arpeggiate line behavior -> I should be able to add
the whole alphabet to "noop" and still have my set keys work for steno.
Does that make sense?

I don't think it make sense, I mean, why even go through the trouble of
adding all those keys to the noop line? Unless of course, we were to
support a more compact way to list those keys (like '[a-z]', or '[:alpha:]'.


Reply to this email directly or view it on GitHub
#292 (comment)
.

@benoit-pierre
Copy link
Member Author

Well, there are 2 separate things:

  • the keymap config format and validation on load
  • the GUI to edit said keymap

I really don't like the current GUI: it would be better if the user could press the keys he want to use when editing an entry. Especially considering that Qwerty names are used, which is confusing when using another keyboard layout.

Something like that:

  • click on entry to change it
  • the code initialize a KeyboardCapture instance, with no suppressed keys
  • when a key is pressed, it's added to the entry set if not already present, otherwise it's removed

In fact, with respect to keyboard layout, I would have preferred for the config to use key codes (and the GUI to display the corresponding key name for the user current keyboard layout).

@morinted
Copy link
Member

morinted commented Nov 7, 2015

Yeah, I have no disagreement there. How would you like to handle the discrepancy that exists there now?

@benoit-pierre
Copy link
Member Author

Check if a key is used more than one time, and display a warning if it is ("key K bound multiple times, behavior is undefined") but keep on going.

@morinted
Copy link
Member

morinted commented Nov 7, 2015

Sure. Would you add that to this PR?
On Nov 7, 2015 4:51 PM, "Benoit Pierre" notifications@github.com wrote:

Check if a key is used more than one time, and display a warning if it is
("key K bound multiple times, behavior is undefined") but keep on going.


Reply to this email directly or view it on GitHub
#292 (comment)
.

@benoit-pierre
Copy link
Member Author

How about merging it and I'll make another pull request for the GUI stuff.

Maybe I'll look into improving the GU and using keycodes too, do you plan on making a release soon? It would be better to get those things right before making a new release, so we don't break compatibility with a released version.

@morinted
Copy link
Member

morinted commented Nov 7, 2015

+2 I'm good with that. Release is not planned short term. We'll probably go
alpha beta rc. Lots of time! I'll merge soon.
On Nov 7, 2015 5:04 PM, "Benoit Pierre" notifications@github.com wrote:

How about merging it and I'll make another pull request for the GUI stuff.

Maybe I'll look into improving the GU and using keycodes too, do you plan
on making a release soon? It would be better to get those things right
before making a new release, so we don't break compatibility with a
released version.


Reply to this email directly or view it on GitHub
#292 (comment)
.

morinted added a commit that referenced this pull request Nov 8, 2015
@morinted morinted merged commit 81f6179 into openstenoproject:master Nov 8, 2015
@benoit-pierre
Copy link
Member Author

WIP version with the keymap conflicts check and better config GUI here: https://github.com/benoit-pierre/plover/tree/keyboard-handling-rework-bis

@benoit-pierre benoit-pierre deleted the keyboard-handling-rework branch February 18, 2016 22:22
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.

None yet

5 participants