dwiel / talon_community

a single source of application-specific scripts
BSD Zero Clause License
121 stars 113 forks source link

basic_keys.keymap list fails to set in context #192

Open dwighthouse opened 5 years ago

dwighthouse commented 5 years ago

I'm working my way through the scripts here, learning what I've missed for the last few months since being away.

In misc/basic_keys.py, for reasons I don't fully understand, the context is loaded with the list of keys from the keymap dictionary, which contains all the other defined dictionary keys save modifier_keys.

https://github.com/dwiel/talon_community/blob/master/misc/basic_keys.py#L140

This results in a failure messages in the console:

2019-10-08 17:51:39    IO cmd {'success': False, 'error': 'error setting list', 'cmd': {'name': 'talon', 'list': 'basic_keys.keymap', 'items': 100, 'cmd': 'g.listset'}, 'ts': 64395175723, 'nonce': 250}

This message seems to repeat every time the file is touched or sometimes just when switching applications (even if neither application is the code editor).

This is curious, as the code of basic_keys.py already has lists for every other meaningful key, as the keymap list is simply the combination of the others. The only use case for keymap as a context list seems to be as one of the selection groups in the get_keys method (where it is redundant).


It appears that the problem has something to do with the fact that context's keymap keys do not have a single instance where the basic_keys.keymap list is used.

Adding the following entry will fix the problem:

{
    ...
    "{basic_keys.modifiers}* {basic_keys.keymap}+": press_keys,
    ...
}

Or it can be fixed by removing the references to the basic_keys.keymap list on lines 94 and 140.

Since the basic_keys.keymap list is just the combination of other lists, it might even be better to remove their lists, and instead use the above entry in place of lines 126-127. This would result in almost identical functionality, except that it would allow the arrow key words (left, right, up, and down) to be used for the arrow keys without any modifiers.

Please explain the reasoning I may have missed behind this decision, and suggest which if any of these possible solutions should be pursued? I can make a PR with the chosen solution if desired.

dwighthouse commented 5 years ago

Meant to check the Slack before posting, but it looks like Jim on Slack found this issue back on September 20th ( #help room ). It doesn't appear to be a Talon-specific issue, and it certainly doesn't harm recognition of other things (only things of that particular list, which in this case is redundant anyway). So, I think any method to reduce unnecessary warnings in the console would be good.

dwiel commented 5 years ago

I apologize for only getting to this now. Is this still an issue? I am not sure i understand what the problem is though. If the issue is that arrow keywords should be available without any modifiers, I think we should make this an option. It's likely that it set up the way it is purposefully. Having the word up in the global context can cause lots of false positives. Maybe we can set it up to be optionally available in the global context?

dwighthouse commented 5 years ago

@dwiel I agree that it's appropriate not to have standard arrow direction words being usable with no modifiers. I would prefer to remove the references to basic_keys.keymap on lines 94 and 140. This would also solve the issue and keep functionality the same. I just wasn't sure the intent. Sounds like your intent is the same as mine, so the removal option is best.

dwiel commented 5 years ago

Cool, sounds good to me.