fmoralesc / vim-pad

a quick notetaking plugin
MIT License
319 stars 22 forks source link

Check if mappings have conflicts (was: Conflicts with vim-go) #57

Closed vendion closed 9 years ago

vendion commented 9 years ago

Both plugins map <leader>s for some function, and I would like to modify the mapping for vim-pad but it doesn't look like the plugin doesn't check if a custom keymap is already set before defining the defaults.

fmoralesc commented 9 years ago

It seems like you've followed the instructions in vim-go's readme, where it recommends to use

 au FileType go nmap <Leader>s <Plug>(go-implements)

It is bad practice to map filetype specific things like this. It should be:

au FileType go nmap <buffer> <LocalLeader>s <Plug>(go-implements)

(vim-pad's mapping on the other hand is global in scope, it starts a global search) If you map it like this, the local mapping should mask vim-pad's.

That said, I'll add some checks.

vendion commented 9 years ago

I know that <LocalLeader> exists, but most of of the plugin docs I have read does not use that, and I'm not quite sure the difference exactly (if you know of a good referance to read that covers the difference between the two that would be great).

fmoralesc commented 9 years ago

Basically, what:help maplocalleader says:

In a global plugin <Leader> should be used and in a filetype plugin <LocalLeader>. "mapleader" and "maplocalleader" can be equal. Although, if you make them different, there is a smaller chance of mappings from global plugins to clash with mappings for filetype plugins. For example, you could keep "mapleader" at the default backslash, and set "maplocalleader" to an underscore.

This is a good convention, and I try to follow it as much as possible. I personally use `asand ",," as`.

I just changed vim-pad mappings, though: regular search is <leader>ss, incremental search is <leader>s<leader> and incremental search without opening new notes is <leader>s!. <leader>s also caused some delay because it waited for the next character to detect the mapping.

fmoralesc commented 9 years ago

I had forgotten, but we already try to check if the mappings are already set, and message the user otherwise. I'm not sure what triggered the original issue now; was the "[vim-pad]: s in normal mode is already mapped" message triggered?

I just pushed a change so vim-pad will silence this message if the user has

 let g:pad#silent_on_mappings_fail = 1

This way, the user can map the failed mapping and silence further messages about it.

Closing for now.

vendion commented 9 years ago

The warning about the mappings did display but looking at the pad.vim source it doesn't check if the user set custom mappings. Making me think that even if I remapped I would still get that warning. One thing I have seen other plugins do is have options like

let g:pad#normal_search_key = ""

and if those are not set, the plugin falls back on its defaults. Maybe using <LocalLeader> instead of <Leader> in my configs will make that unneeded, but it does seem like a good way to override the settings.

Either way the silent mapping flag is welcome for now.

fmoralesc commented 9 years ago

vim-pad performs that check by using the <unique> flag when actually mapping, see here. If a previous mapping is found, it won't map and throw the message. The silent flag simply prevents the message to be displayed, assuming the user will have a custom mapping.

Anyway, adjusting the mappings like that is not a bad idea, I'll look into it.

fmoralesc commented 9 years ago

Just pushed an implementation of that method. The maps can be configured independently now. ;)