Don't override existing mappings #202
Conversation
|
@EinfachToll Any thoughts on this? |
|
i'd prefer vimwiki to use |
|
@Ram-Z , Thank you for this PR! While this removes an annoying and cryptic error message, it also makes Vimwiki appear to fail silently if a user has already mapped one of its keys. In many circles of software development, silent failure is considered a debugging nightmare. How would you feel about instead outputting a more clear error message when the user has already mapped one of Vimwiki's keybindings? One example would be "Vimwiki failed to map |
|
In the interest of keeping a clean repository, I am closing this PR due to lack of response from the author. If there is renewed interest in making errors more clear when a there is a mapping conflict, please feel free to ping on this PR or create a new one. |
|
The problem with displaying any message in vim is that it gets in the way of starting vim. This patch was trying to address that issue, printing a different message defeats the point. I don't think that any user will have troubles debugging this. It's not like it will not do anything at all when the binding is already mapped, it will instead execute the mapping the user has selected (or was provided by a different plugin). A better approach would be to have a way to disable setting some (or all) mappings. |
|
@hq6 Regarding this PR, I'm happy to leave it closed if this solution is not suitable for you, but I'll probably keep the patch alive on my branch as this is essential for me to use vimwiki. |
|
This appears to be a reasonable argument for why the user will not have trouble debugging this. I was thinking about silent failure in the context of debugging larger, long-running systems, but it is a valid point that the existing mapping would do something so the failure would not be silent. @ranebrown @Nudin If you either you also think this PR is a good idea, I'm happy to merge it. |
|
I don't have any problems with this solution even through I'm not sure if it is the best solution. |
|
Thanks. 👍 |
Currently, if a mapping for
<Leader>ww(for example) already exists, thenmapwill echo an error message because of the<unique>argmuent and not set the mapping. This patch silences this error.