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

Execute asynchronously and allow adding extra arguments #9

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ferranpm
Copy link

I wanted to run one instance of Vim and switch between both editors without having to close Vim every time.
To achieve this I've added the synchronous checkbox to use exec instead of execSync and the extraArgs field to add --servername vscode --remote-silent to the parameters so only one instance of Vim is used.

I've just tested it with MacVim but seems to work nicely.

@jonsmithers
Copy link
Owner

This looks nice! Thanks for figuring out the sync/async thing. I can confirm it works on linux with gvim too.

I have some reservation about the Extra Args option since it would open up a new class of hard-to-debug user problems. Examples: what happens when the user puts " characters in there, or a simple alphanumeric string, which vim sees as a file to open. The error reporting for these problems is very lacking. What kind of use cases does Extra Args benefit?

@ferranpm
Copy link
Author

What kind of use cases does Extra Args benefit?

The only other useful options I see for Extra Args are:

  • -u/-U to load a different vimrc/gvimrc when running from this plugin
  • --cmd to run an arbitrary command
  • Maybe also -S to load a session file (I'm not sure about this one since I'm not a session files user)

Further improvements may include being able to add some variables to the text field to run an instance of Vim per project with something like --servername ${projectPath} --remote-silent.

Nonetheless I understand your concerns and if you prefer to keep it more controlled I can remove the text field and add another checkbox for Single instance, but in that case we should remind users that checking it also requires to uncheck the synchronous one.

@huyz
Copy link

huyz commented Nov 7, 2022

Would love this feature merged in

@jonsmithers
Copy link
Owner

There are two separate features in this PR, and I'm not sure which @huyz is referring to.

It seems that this PR doesn't currently work. When I run it locally and don't set the Extra Args setting, it interpolates null into a string for shell execution.

@huyz
Copy link

huyz commented Nov 11, 2022

I was referring to the async part.

@ferranpm
Copy link
Author

It seems that this PR doesn't currently work. When I run it locally and don't set the Extra Args setting, it interpolates null into a string for shell execution.

My bad, this last commit should fix this issue

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