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 enbale_start_key_flag #19

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

Conversation

omrisarig13
Copy link

@omrisarig13 omrisarig13 commented Jun 3, 2019

@simeji - I noticed that there weren't any way to disable the start key of the plugin. Meaning, that the plugin always was remapped to a given key in vim.
I tried to set the key to none (by using let g:winresizer_start_key = '') but then the plugin had an error when it tried to run the nnoremap function.

To fix this, I added a new flag to the flags of the plugin - g:winresizer_enable_start_key. This key is on by default, so the behavior of the plugin stays the same. But, if someone wouldn't want the plugin to run over any vim key (like I did), he would be able to do it by setting this flag to 0 (let g:winresizer_enable_start_key = 0), and the remap function won't happen.

I think I added all the doc needed for the new flag (both in the doc file and in the README file).

@simeji
Copy link
Owner

simeji commented Jul 20, 2019

@omrisarig13 Thanks your PR, and so sorry about my late response.
I think that this problem is inconvenient and would like to fix that problem as soon as possible.

I read your PR.
How about you just don't map start_key when g:winresizer_enable is 0 without adding winresizer_start_key_enable?

@omrisarig13
Copy link
Author

@simeji - I don't think that this would solve the issue as I have experienced it.

In my vim configuration, I wanted to have functionality of this plugin, but I didn't want to have specific mapping that would activated it (I want to activate it manually by the command).
Therefore, I would like to have the plugin enabled (and set g:winresizer_enable to 1).

However, I you want to avoid the extra key, I can check if the key is empty before mapping it. This should make the error disappear as well. How does that sound?

@simeji
Copy link
Owner

simeji commented Jul 24, 2019

@omrisarig13
Please excuse my lack of explanation.
I've mentioned the direction of modification of the code.
For example, like this

if g:winresizer_enable != 0
  exe 'nnoremap ' . g:winresizer_start_key .' :WinResizerStartResize<CR>'
endif

The plugin maps the some keys to run the commands of the plugin
automatically by themselves.
Until now, those commands were always mapped, regardless of the state of
the plugin and the mapping themselves.

This caused two problems:
* When the plugin was not enabled the mapping still existed.
* When the mapping was empty, an error massage was generated.

To fix both of those problems, I added two tests before running the
mappings: first, it checks that the plugins is indeed enabled, and then
it checks that the mapping is not empty. Only in case both of those
tests passes, the commands are mapped.
@omrisarig13
Copy link
Author

@simeji - I just reread my original PR and saw that I was very unclear there about the use case that I thought as a problem.

I wanted to have the functionality of the plugin, but I didn't wanted to have a mapping to run it, I wanted to always call it manually (by running the wanted command). To do this, I tried to set the start key to '', but vim raised an error every time I opened it (about having a map command with empty value).

The problem you understood might be a problem as well, but it wasn't the one I have experience.

To fix both of those problems, and avoid adding any more configuration to the plugin, I added two tests, instead of the one you have proposed.
The first one is the one you proposed, to validate that the plugin is enable before running the mapping command. The second test was to validate that the key is not empty, so the command won't run in case the key is not present (which raises an error).

I hope that this fix solves both of the problems.

@davidsierradz
Copy link

Maybe use :help <Nop>?

let g:winresizer_start_key = '<NOP>'

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.

3 participants