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

Add support for setting custom mappings #93

Closed

Conversation

mdedonno1337
Copy link

@mdedonno1337 mdedonno1337 commented Mar 6, 2021

This PR implements the functionnality to set custom mappings.
This option can be used by setting the g:bullets_bindings variable in the vim rc file as follow (for example):

let g:bullets_bindings = [
    \['inoremap', '<cr>', '<C-o>:InsertNewBullet<cr>'],
    \['nnoremap', 'o', ':InsertNewBullet<cr>'],
    \['nnoremap', '<C-t>', ':ToggleCheckbox<cr>'],
\]

Note that the ToggleCheckbox is binded to Ctrl-T in this case for example.
This allows to set only a part of the bindinds, as the users whiches.

Moreover, the doc is not set at the moment but should be done before merging if this feature is OK for you.

@mdedonno1337 mdedonno1337 marked this pull request as draft March 6, 2021 18:10
@mdedonno1337
Copy link
Author

Base upon the failed test of the commit 15a35b5, I dont know how to allows the users to use the inset_new_bullet() function from the outside. I think we should eigher expose the insert_new_bullet() function to the exterior or add a second function to expose it.

I had issues with Coc (conquere of completion) with the autocomplete (also binded to the key), and solved it with:

let g:bullets_set_mappings = 1

let s:bullets_bindings = [
  \['nnoremap', 'o',         ':InsertNewBullet<cr>'],
  \['vnoremap', 'gN',        ':RenumberSelection<cr>'],
  \['nnoremap', 'gN',        ':RenumberList<cr>'],
  \['nnoremap', '<leader>x', ':ToggleCheckbox<cr>'],
  \['inoremap', '<C-t>',     '<C-o>:BulletDemote<cr>'],
  \['nnoremap', '>>',        ':BulletDemote<cr>'],
  \['inoremap', '<C-d>',     '<C-o>:BulletPromote<cr>'],
  \['nnoremap', '<<',        ':BulletPromote<cr>'],
  \['vnoremap', '>',         ':BulletDemoteVisual<cr>'],
  \['vnoremap', '<',         ':BulletPromoteVisual<cr>']
\]

" This is the "normal" binding for Coc, adapted for bullets
inoremap <silent><expr> <CR>
  \ pumvisible() ? "\<C-y>" :
  \ "<C-]><C-R>=bullets#insert_new_bullet()<cr>"

with the exposed function in the bullets.vim file:

fun! bullets#insert_new_bullet()
  return <SID>insert_new_bullet()
endfunc

This PR was originally to solve this issue and let the people customize the configuration; I should probably split the PR in two, as you prefere.

@dkarter
Copy link
Member

dkarter commented Mar 8, 2021

Hi @mdedonno1337, thank you for submitting this PR! This is something that was requested by a few people due to conflicts with other plugins, so I'm sure it will be very helpful for a lot of people and hopefully motivating! (#43, #77, #57, #85, #74)

AFAIK, the idiomatic way to expose a mapping from a plugin and allow for user customization is using <Plug>. This allows us to change the implementation of inserting a bullet in the future without all the users having to update their vimrc for the change.

For example, in the plugin we will have:

inoremap <silent> <Plug>(bullets-insert-new-bullet) <C-]><C-R>=<SID>insert_new_bullet()<cr>

let s:bullets_bindings_default = [
  \['imap',     '<cr>',      '<Plug>(bullets-insert-new-bullet)'],

Then in their vimrc users can add:

imap <silent><expr> <CR> pumvisible() ? "\<C-y>" : "\<Plug>(bullets-insert-new-bullet)"

(notice that I switched inoremap to imap whenever using the <Plug> mapping. This is necessary when using <Plug> mappings)

Here are some really good articles highlighting the use of <Plug> and the benefits of using it:

Here are some other plugins using <Plug> mappings for inspiration:
image

Maybe it would be better if instead of exposing a 2D array for the user to customize, which locks them into the specific ways we use mappings (<silent> <buffer> etc..), we instead expose multiple <Plug> mappings that they can use however they like. (We should still do the initial mappings by default though.)

@mdedonno1337
Copy link
Author

Thanks for the feedback!
I will check that out and make the modifications.
I will force push the new branch here, so we can keep the discussion if it's ok for you.

@megalithic
Copy link

really happy to see this work being done. any updates @dkarter or @mdedonno1337?

@mdedonno1337
Copy link
Author

mdedonno1337 commented Apr 23, 2021

really happy to see this work being done. any updates @dkarter or @mdedonno1337?

Thanks for the interest! Still on my todo list...
If you have any feedback, let us know.

@pwntester
Copy link

looking forward for this, thanks for the PR! 🚀

mmrwoods added a commit to mmrwoods/dotfiles that referenced this pull request Dec 28, 2021
Rework insert mode cr mapping to accept completion if menu visible

See bullets-vim/bullets.vim#43
And bullets-vim/bullets.vim#93
@wenzel-hoffman
Copy link
Contributor

wenzel-hoffman commented Sep 10, 2022

Oh, I didn’t notice this one existed before making #128. I’m surprised I implemented that in a very similar way. I guess it’s a good sign 😅 ?

@wenzel-hoffman
Copy link
Contributor

@dkarter I guess this can be closed now since #128 already covers what this change was trying to achieve?

@dkarter
Copy link
Member

dkarter commented Sep 14, 2022

I agree, @wenzel-hoffman! Closing this for now @mdedonno1337 if the current solution doesn't fit your need we can discuss further. Thanks for your efforts!

@dkarter dkarter closed this Sep 14, 2022
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.

None yet

5 participants