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

Support 'code -rw' in running instance and wait until tab gets closed #24327

Closed
azixMcAze opened this issue Apr 8, 2017 · 16 comments
Closed
Assignees
Labels
feature-request Request for new features or functionality on-testplan

Comments

@azixMcAze
Copy link

Hello

When opening a file from the command line with code --wait --reuse-window <file>, the command waits for the previously opened window to be closed before exiting. It is coherent with the use of --wait without --reuse-window but it is not very convenient.

One can want to reuse an existing window because it is faster than opening a new window. The small delay before the file has finished opened can be a little annoying when writing a commit message for example (if you don't use git or the upcoming SCM API).

One can also argue that it is easier to only close a tab than close the whole window : cmd+w instead of cmd+w cmd+w or cmd+shift+w on mac for example

Or maybe there could be a new command-line toggle instead of modifying the behaviour of --wait

@chrmarti
Copy link
Collaborator

A new parameter would make the most sense.

@chrmarti chrmarti added the feature-request Request for new features or functionality label Apr 10, 2017
@bpasero bpasero removed their assignment Apr 10, 2017
@chrmarti chrmarti added this to the Backlog milestone Apr 17, 2017
@henvic
Copy link

henvic commented May 8, 2017

Just had the same issue while trying to use Visual Code as my git editor.

And yes, a new parameter would be more appropriate.

@kaiwood
Copy link
Contributor

kaiwood commented May 19, 2017

Hmm, why a new parameter, am I overlooking something? 🤔

--wait exists so that code works as an editor for stuff like git messages. I see no reason why it would wait for the window at all, because from the the users perspective the whole editing process is finished as soon as I close the active editor.

@chrmarti
Copy link
Collaborator

--wait is very likely already in use in a way that depends on it waiting for the window. Having a new parameter waiting for the editor tab to close would also allow for reusing the current window, which would be nice because it keeps the current context and is also faster than opening a new window.

@kaiwood
Copy link
Contributor

kaiwood commented May 19, 2017

Still not convinced if that is correct behaviour for -w I'd expect, but yea, I've seen a lot of people around here that expect all kind of "weird" things, especially if something is changed between releases. So you might be right :)

I'm willing to take a look into the implementation to check off something from my personal itch-to-scratch-list.

Couple of questions/thoughts:

To not clutter the CLI to much, would changing the behavior only if -wr is used make sense? Would a setting be appropriate?

If it has to be a separate command line argument, any suggestions how to name such a thing in a short and descriptive way?

@chrmarti
Copy link
Collaborator

Awesome! I think the same user might want to use either behavior, depending on the task/goal. For that I would add a new CLI parameter and not a setting. What about --wait-file: "Wait for file to be closed before returning."?

@jbrodriguez
Copy link

I took a summary look at the code that implemented "-w", but it seemed to me like a "simple" change, since it used node's process behavior.

I think this feature is a bit more complex since it needs to tap into the vscode api, to look for a "ClosedTab" (or such) event.

@kaiwood hope you can get it done 👍

@bpasero bpasero changed the title Wait for tab closed instead of window when opening file with 'code -rw' Support 'code -rw' in running instance and wait until tab gets closed Jun 29, 2017
@bpasero
Copy link
Member

bpasero commented Jun 29, 2017

This is not easy to accomplish due to the multi-process architecture that we have:

  • user starts the command line process via code --wait
  • main process is started
  • renderer process is started for a new window or an existing window is reused
  • editor closes inside renderer
  • renderer needs to notify main process
  • main process needs to notify command line process

On top of that, if Code is already running, we have a launcher process that talks to the main process to do things and so we need to wire this event even through from main process to launcher process.

@bpasero
Copy link
Member

bpasero commented Jun 29, 2017

A better approach then trying to communicate through all the processes is probably to create a random file that the starting command line process watches for deletes and let the window delete this file when the appropriate action has been taken (e.g. editor closes).

@DAddYE
Copy link

DAddYE commented Jul 17, 2017

Any update on this? Right now is quite difficult to use vscode as git editor. Thanks!

@chrmarti chrmarti added the help wanted Issues identified as good community contribution opportunities label Jul 17, 2017
@chrmarti
Copy link
Collaborator

@DAddYE Not sure I will get to it anytime soon. Contributions welcome :)

@ollwenjones
Copy link

I still use sublime as my git editor for this very reason. It's always hanging about in the background just for that (and occasional other tricks.) 🙂 Not to complain, but just to say there's definitely interest for this.

@DAddYE
Copy link

DAddYE commented Aug 16, 2017

@chrmarti any instructions how to get started in achieving this feature?

@chrmarti
Copy link
Collaborator

@DAddYE You could look into how this is done for --wait which just waits for the window to close. That seems to create a file it waits on to be deleted and passes the file path on as a command line argument here: https://github.com/Microsoft/vscode/blob/master/src/vs/code/node/cli.ts#L67

It is the main process waiting for the window to close that then deletes the file. The same won't work for waiting on a file to close because the main process doesn't get notified when that happens. (https://github.com/Microsoft/vscode/blob/master/src/vs/code/electron-main/windows.ts#L389)

So maybe the main process should still get a file path and then just forward that to the renderer process that shows the file and can listen on the close event. This is where the file to open in an existing window is sent to that window and here you could pass the file path along: https://github.com/Microsoft/vscode/blob/master/src/vs/code/electron-main/windows.ts#L615

There will be other cases to consider: When there is no window open yet. When a new window needs to be opened, but there are other windows.

Does that help you get started? There is some code reading involved here, I'm not familiar with all the parts either.

@bpasero bpasero assigned bpasero and unassigned chrmarti Sep 11, 2017
@bpasero bpasero removed the help wanted Issues identified as good community contribution opportunities label Sep 11, 2017
@bpasero bpasero added this to the September 2017 milestone Sep 11, 2017
@bpasero bpasero removed this from the Backlog milestone Sep 11, 2017
@bpasero
Copy link
Member

bpasero commented Sep 11, 2017

--wait now works in a way that it will return when all of the provided paths are closed in the window. This works for either a window opening for the files or when opening the files within a running instance (e.g. by providing -r option).

@jbrodriguez
Copy link

Just wanted to say thank you for having implemented this.

I now use vscode 95% of the time.

I use Atom the other 5% for the git merge interface, but don't keep it open anymore.

Awesome job !

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

No branches or pull requests

8 participants