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

fix :wq in remote #3998

Merged
merged 1 commit into from Aug 27, 2019
Merged

Conversation

stevenguh
Copy link
Contributor

What this PR does / why we need it:
Fix the issue where :wq can't close a file in vscode remote

Which issue(s) this PR fixes
#3922

@J-Fields
Copy link
Member

Thanks!

@J-Fields J-Fields merged commit dbe79fb into VSCodeVim:master Aug 27, 2019
@stevenguh stevenguh deleted the feature/fix-wq-in-remote branch August 27, 2019 04:26
@suo
Copy link
Contributor

suo commented Aug 28, 2019

Hi @J-Fields, this change is a regression for me (I actually removed the await in a previous PR to fix the following issue: #3777).

This change causes all saves to block the Vim extension until the save is completed. With the remote development extension, this save can actually take multiple seconds, depending on the connection quality and file size (see the above issue for an example).

I would argue that completely blocking the Vim extension (no shortcuts, no ex commands, etc.) is worse than failing the save in this instance. Especially since :w is likely more common than :wq. Is it possible to revert this PR?

I suppose the ideal solution is to have sequential commands await each other. Or for whatever state machine handles the vim state not block on an ex command finishing. Happy to implement something along those lines if there's consensus on the right path.

cc @jpoon @Chillee

@J-Fields
Copy link
Member

@suo Thanks for pointing this out. I think the solution might be to await the save only for :wq, to avoid the race condition while not blocking for :w.

@stevenguh
Copy link
Contributor Author

stevenguh commented Aug 29, 2019

Ok. I see where you are coming from. I assumed that's the cost working on remote, because all other write commands like :wa, and :wqa are blocking as well.

@J-Fields That's what I am thinking as well. I am looking into the code. It seems like we can do that.

This was referenced Aug 29, 2019
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

3 participants