donniebreve / touchcursor-linux

TouchCursor style keyboard remapping for Linux.
GNU General Public License v2.0
133 stars 28 forks source link

Added permanent key mappings #47

Closed donniebreve closed 2 years ago

donniebreve commented 2 years ago

Added functionality for permanent key remapping. See #15.

donniebreve commented 2 years ago

I would like to see a better explanation of what remap does in the configuration file (as it is with the other table headers ([...])).

What would you like to see as a comment?

I think that for clarity, it is important to have the remap table before the hyper key and hyper key bindings tables in the config file to emphasize the independency of permanent mappings to the hyper key.

I am wondering if this is just confirmation since that's how the config file is.

Furthermore, I think that it is worth mentioning in the config file that if you want to define a specific behaviour to a permanently remapped key (KEY_T) with a hyper key, you will get a behaviour of a remapped key (KEY_M) when pressing the original key (t): When pressing hyper key + t, you get e. I am not sure if this is desirable or whether it would be better to allow explicitly defining a different hyper key behaviour for KEY_T: To get r when pressing hyper key + t and to get e when pressing hyper key + m.

In the same manner, I feel like there should be an option to use a permanently remapped key (t in this case) as a hyper key. As of now, that is not possible.

It seems like you're conceptualizing this in a different way than I did. You're expecting that 't' is always recognized as 't' for input, regardless of the configuration. Is it more intuitive to always refer to a key as the physical keyboard key, regardless of remapping?

I thought of remapping as if it were an external application. If you remap 't' to 'm', you shouldn't expect 't' as input anymore. If you remap 't' to 'm', but then want to map 't' to 'y' when space is held, you would remap 'm' to 'y' for the layer.

I wrote this with the remap layer being the first layer, and I believe you're imagining it as the last layer.

How do other key mapping projects (xkb, xmodmap) handle remapped keys. Do later configuration lines still detect the key as 't'?

I'm open to reworking this, although the proposed logic is a bit more difficult to implement. The logic change should allow you to use a remapped key as the hyper key but you would refer to it as the physical key in the hyper configuration.

Proposed example:

[Hyper]
Hyper1=KEY_SPACE

[Bindings]
KEY_T=KEY_E

[Remap]
KEY_T=KEY_M
KEY_E=KEY_B
  1. Pressing 't' outputs 'm'
  2. Pressing space+'t' outputs 'e'
  3. Pressing 'e' outputs 'b'
auouymous commented 2 years ago

"Permanent" remapping sounds like it modifies something so the key is forever remapped even if touchcursor is removed.

How do other key mapping projects (xkb, xmodmap) handle remapped keys. Do later configuration lines still detect the key as 't'?

Xmodmap only binds symbols to keycodes. For keycode 28 = t T and keycode 58 = m M, the physical t key always produces keycode 28 even if you change it to keycode 28 = m M. X11 programs that don't read the symbols will bypass any mapping done by Xmodmap.

The proposed example seems the most logical, it produces exactly what you see in the file.

donniebreve commented 2 years ago

Xmodmap only binds symbols to keycodes.

After some cursory research, I noticed this too. I will move forward with the proposed changes, i.e. moving the remap layer to the last layer.

"Permanent" remapping sounds like it modifies something so the key is forever remapped even if touchcursor is removed.

Any suggestions for different wording?

auouymous commented 2 years ago

Any suggestions for different wording?

Drop the word "permanent" and only call it key or keycode remapping.

Adda0 commented 2 years ago

I am wondering if this is just confirmation since that's how the config file is.

Just a confirmation, yes.

Any suggestions for different wording?

Drop the word "permanent" and only call it key or keycode remapping.

I would rather keep the Permanent remappings as the official name for the feature (and therefore keep this in the config file). What I was thinking about is more in the terms of adding additional explanation with examples. Something as follows:

# Permanent remappings.
# Allows to set permanent remappings when running the service without a need to hold a hyper key.
# Each remapping will be active permanently when the service is running.
# The permamently remapped key can be remapped differently while holding the hyper key in table `[Bindings]` with the original key name:
# [Remap]
# KEY_T=KEY_M
# [Bindings]
# KEY_T=KEY_D
#
# You can swap the functionality of two keys as follows:
# [Remap]
# KEY_T=KEY_M
# KEY_M=KEY_T 
[Remap]

If we want to further discuss the exact wording, I can create a PR to be merged onto this branch when we are in an agreement. Actually, looking at the config file now, We have two separate tables allowing setting some remappings ([Remap] and [Bindings]). We should probably expand the description of table [Bindings] as well, otherwise users may not understand that the bindings in the table [Bindings] have an effect only when holding a hyper key. Something as follows for the table [Bindings] then:

# Hyper key bindings.
# The following bindings remaps keys only when holding a hyper key.
# The remapping has the format:
# KEY_1=KEY_2
#
# For usable key names, see 'github.com/torvalds/linux/blob/master/include/uapi/linux/input-event-codes.h'.
#
# For example, when holding the hyper key, key 't' will output key 'k':
# [Bindings]
# KEY_T=KEY_K
[Bindings]

Xmodmap only binds symbols to keycodes.

After some cursory research, I noticed this too. I will move forward with the proposed changes, i.e. moving the remap layer to the last layer.

Both approaches are possible. I think, however, that having the permanent remappings as the last layer would be more beneficial to the user when the original keys are to be remapped with the hyper key functionality as well. I agree with the chosen approach. Exactly as @donniebreve described, I prefer the following approach much more for the ease of use for the user:

The logic change should allow you to use a remapped key as the hyper key but you would refer to it as the physical key in the hyper configuration.

auouymous commented 2 years ago

Those three comments read exactly the same if "permanent" is removed.

# Allows to set remappings when running the service without a need to hold a hyper key.
# Each remapping will be active when the service is running.
# The remapped key can be remapped differently while holding the hyper key in table `[Bindings]` with the original key name:
Adda0 commented 2 years ago

Those three comments read exactly the same if "permanent" is removed.

Maybe, but if you want to refer to the feature anywhere else without all these exhaustive comments, you have to have a single self-explanatory name for the feature. Without the permanent adjective, we have two different features present in the config file: bindings and bindings. That does not help us determine which is which, does it? One is for the hyper key bindings and the second is for permanent bindings. I think using permanent makes sense and describes pretty precisely what they are. And no one will think that when the service is not running, the new “permanent” bindings will be active, I believe. And if they do, they have the comments to explain it.

Furthermore, due to backward compatibility, we cannot rename the hyper key bindings under the table [Bindings] unless we want to break everyone's configs. Therefore, we have to add a specific, distinguishable name each time we introduce a new feature (new types of bindings).

Adda0 commented 2 years ago

@donniebreve What do you think about this PR? I have been using the permanent remapping since opening this PR and I think it works great. I cannot say that I found any problems with it.

As you mentioned somewhere, it might be a good idea to review the whole of mapper.c to probably fix and improve the emitting logic, but that would be a task for another PR.

As far as this PR goes, I think it is a great addition to the application and is ready to be merged, except for maybe addressing the debug prints.

donniebreve commented 2 years ago

Sure, let's merge this in until I get some motivation to rewrite some things.