sodapopcan / vim-twiggy

Git branch management for Vim
280 stars 15 forks source link

[WIP] Allow users their original keymaps #33

Open aileot opened 4 years ago

aileot commented 4 years ago

Introduce g:twiggy_keymaps_on_branch and g:twiggy_keymaps_to_sort for the present.

The reason to avoid simple g:twiggy_keymaps is that it's a bit complicated for users to leave comments in list especially who uses an older version of vim.

" TODO: before merging to 'master', make keymaps configurable w/o magic numbers " For example, " 'gP': 'Push', ['REMOTE'] " '!P': 'Push', ['--force']

If you accept this feature, just tell me so without merge to avoid excess deprecation notice.

sodapopcan commented 4 years ago

Hi there! Very sorry for the long delay in reply. I have been focusing on non-programming projects outside of work and twiggy has suffered a bit for it.

I have wanting to add this feature but the work really comes from exactly what you pointed out: how to make the config work nicely.

I have some ideas on this but as I start to type them out, I realize I need to think them through a bit more. But I think there needs to be tokens for each individual action. Something along the lines of:

let g:twiggy_mappings["gP"] = "force_push"

Another idea I was toying with is to use the existing mappings, though that could be confusing. So:

TwiggyRemap("gP", "<leader>gP")

...which would remap gP to <leader>gP

I'm open to other ideas.

aileot commented 4 years ago

Thank you for the reply.

It seems better to map keys via dictionaries so that we don't have to see anywhere but a dictionary in our own vimrc if we either want to change some mappings or forget the mappings. (We can postpone an update for quickhelp on user's mappings)

On the other hand, remapping could confuse us, as you said. It could cause recursive mappings, and needs some extra functions for users who want to swap some default mappings.

Addition to that, we should avoid magic-number-like named settings.

How about a format like below?

let g:twiggy_mappings = {
     \ ...
     \ 'm':  'Merge',
     \ 'M':  'Merge remote',
     \ 'gm': 'Merge --no-ff',
     \ 'gM': 'Merge remote --no-ff',
     \ ...
     \ }

Or it may be better to accept only lists to indicate they're only tokens for Twiggy.

let g:twiggy_mappings = {
     \ ...
     \ 'm':  ['Merge'],
     \ 'M':  ['Merge', 'remote'],
     \ 'gm': ['Merge', '--no-ff'],
     \ 'gM': ['Merge', 'remote', '--no-ff'],
     \ ...
     \ }