Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Allow users their original keymaps #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aileot
Copy link

@aileot aileot commented Feb 20, 2020

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.

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
older version of vim.
@aileot aileot changed the title Allow users their original keymaps [WIP] Allow users their original keymaps Mar 20, 2020
@sodapopcan
Copy link
Owner

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
Copy link
Author

aileot commented May 3, 2020

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'],
     \ ...
     \ }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants