key-bindings: overwrite collision with a log warning (with test coverage) #432

Explicitly setting key bindings can collide with default key bindings and throw an error.

This code makes an update such that now defining a keybinding will succeed but print a warning, as this could indicate a mistake. If you don't want the warning, you can explicitly unmap and re-map the binding.

It can be tested by adding the same keybinding to your config file twice. The later one should succeed with a warning.

This is a continuation of #429, with test coverage added.

Behavior Before

• Rebinding a key would not result an error

Behavior After

For example, with this in fuzzel.ini:

execute=y
execute=z

Could result in:

➤ fuzzel
warn: config.c:309: unmapping default keybinding: execute: was previously triggered by Return
warn: config.c:309: unmapping default keybinding: execute: was previously triggered by KP_Enter
warn: config.c:309: unmapping default keybinding: execute: was previously triggered by Control+y
warn: config.c:314: unmapping keybinding defined at /home/mark/.config/fuzzel/fuzzel.ini:2: execute: was previously triggered by y

@dnkl I've run the tests several times to make sure they reliably pass and they do.This is ready for final review/merge.

Rebased and resolved conflict.

@Cyclic4179 The goal of this PR was deal with key collision -- different actions mapping to the same key-- but it also affects a second case, generating new warnings which can't be turned off. That's Action rebinding. For example, if you want to replace all execute= bindings with execute=y, that was going to cause new warnings that can't be disabled.

I'm going to push a new commit to narrow the scope to what was stated in the title: "Key Collision". I'll illustrate some different keybindings and how they will work after this PR:

execute-or-next=KP_Enter

This will generate an warning, because KP_Error is bound to execute by default:

fuzzel.ini:3: [key-bindings].execute-or-next: KP_Enter previously mapped to 'execute', overwriting

This warning can be eliminated by clarifying the intent by specificing exactly what you would like bind execute to:

# KP_Enter from the list-- no more conflict with KP_Enter and no error
execute=Return Control+y
execute-or-next=KP_Enter

Now here's a a case of Action Rebindings, which generated several warnings original that couldn't be disabled, but now generates no warnings. What's happening here is that the default bindings of execute have been replaced with y:

execute=y

This is also less clearly a mistake. You have explicitly declared what you want to map execute= to in this case, which expresses a much clearer intent than the key-collision case, which could be a well-intentioned accident to bind the actions to the same key.

@dnkl I've rebased this, I believe it's good to merge and ready for a final review.

I'll need to think about this, and review after the upcoming release.