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

Open + Clone in VS Code not properly cloning #73101

Closed
ColbyTresness opened this issue Apr 30, 2019 · 21 comments
Closed

Open + Clone in VS Code not properly cloning #73101

ColbyTresness opened this issue Apr 30, 2019 · 21 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug linux Issues with VS Code on Linux verified Verification succeeded windows VS Code on Windows issues workbench-os-integration Native OS integration issues
Milestone

Comments

@ColbyTresness
Copy link

ColbyTresness commented Apr 30, 2019

  • VSCode Version: 1.33.1
  • OS Version: Windows

Steps to Reproduce:

I have a webpage that's essentially a repository of relevant git repos, and we're adding an "open in VS Code flow". I'm linking using:

"vscode://vscode.git/clone?url=[url]"

On windows, if VS Code isn't already open, clicking this button will open VS Code, but will not successfully clone the repo. If VS Code is open, then clicking this button works properly. This works properly on a Mac.

@vscodebot vscodebot bot added the git GIT issues label Apr 30, 2019
@RMacfarlane RMacfarlane added the windows VS Code on Windows issues label Apr 30, 2019
@ColbyTresness
Copy link
Author

We tried this in Ubuntu today and it didn't work either.

@joaomoreno
Copy link
Member

Let me summarize:

  • On both Windows and Linux this works if Code is already running. It doesn't work in neither platform if Code isn't already running.
  • On macOS everything works as expected, regardless.

Did I get that correctly?

@joaomoreno joaomoreno added info-needed Issue requires more information from poster linux Issues with VS Code on Linux workbench-os-integration Native OS integration issues and removed git GIT issues labels May 6, 2019
@ColbyTresness
Copy link
Author

Yes, that’s correct.

@joaomoreno joaomoreno added bug Issue identified by VS Code Team member as probable bug and removed info-needed Issue requires more information from poster labels May 7, 2019
@joaomoreno joaomoreno added this to the Backlog milestone May 7, 2019
@ColbyTresness
Copy link
Author

Any update here? This flow seems completely broken, and we've been waiting for over a month @joaomoreno

@joaomoreno
Copy link
Member

No update. Feel free to investigate and submit a PR for this.

@joaomoreno joaomoreno added the help wanted Issues identified as good community contribution opportunities label Jun 12, 2019
@chrisdias
Copy link
Member

It doesn't work on Mac for me either... Code has to be open for clone to work.

@YisraelV
Copy link
Contributor

I am looking into this.

@YisraelV
Copy link
Contributor

@joaomoreno the problem is the class ExtensionUrlHandler registers itself as url handler upon instantiation, and it's instantiated too late.

The solution is probably to instantiate it earlier, but finding the appropriate place to do this is too difficult for me.

@ColbyTresness
Copy link
Author

So who can we contact who can help out? We'd like to use this flow more, not less, and as it stands it's still broken.

@YisraelV
Copy link
Contributor

@joaomoreno, just to clearify:

When we receive the request to open the url, the extension (Git) hasn't registered a handler yet.

The class I linked to is supposed to buffer url requests until the extension is active. Problem is it's not registered itself yet. It's only registered upon instantiation.

The instantiation happens when the class is injected (mainThreadUrls) There is probably a way to to cause early instantiation, but how and where to do it is beyond me without some guidance.

@joaomoreno
Copy link
Member

joaomoreno commented Jun 26, 2019

When we receive the request to open the url, the extension (Git) hasn't registered a handler yet.

It doesn't have to. Every vscode://EXTENSION.ID/* URI will automatically activate the extension with id EXTENSION.ID. Unless there's a bug in that mechanism, but the implementation is there: https://github.com/Microsoft/vscode/blob/d0c362792aa27ae6b6648f8b99d8d78e6095d537/src/vs/workbench/services/extensions/common/inactiveExtensionUrlHandler.ts

@YisraelV
Copy link
Contributor

@joaomoreno for the code you linked to to run the class has to first register itself with the urlService, which it does in the constructor (here). When the request arrives the constructor hasn't run yet.

@joaomoreno
Copy link
Member

Possibly broken by 819dda9 cc @jrieken

@jrieken
Copy link
Member

jrieken commented Jun 27, 2019

Possibly broken by 819dda9 cc @jrieken

How? That commit renames a file and appends a new line?

@joaomoreno
Copy link
Member

🤦‍♂️ You're right. I meant this: 900d58b cc @bpasero

@bpasero
Copy link
Member

bpasero commented Jun 27, 2019

@joaomoreno unlikely, I do not see anything changing. Previously you would do:

serviceCollection.set(IExtensionUrlHandler, new SyncDescriptor(ExtensionUrlHandler));

and now it is:

registerSingleton(IExtensionUrlHandler, ExtensionUrlHandler);

It is now actually even called earlier than before.

@joaomoreno joaomoreno modified the milestones: Backlog, July 2019 Jun 28, 2019
@joaomoreno
Copy link
Member

@bpasero I'm pretty sure this is what broke it. As a fix, I was forced to change registerSingleton into registerWorkbenchContribution with LifecyclePhase.Ready since on the main side since Ready is the only state propagated to the main side. If more window states would propagate to the main side, we could change it to other another state, ie Eventually.

@joaomoreno
Copy link
Member

Argh... reverted... this needs to be a service. 🤦‍♂️

@joaomoreno joaomoreno reopened this Jul 31, 2019
@joaomoreno joaomoreno reopened this Jul 31, 2019
@joaomoreno
Copy link
Member

It is now actually even called earlier than before.

@bpasero I don't think this is true at all. My singleton ctor is running well after the lifecycle Ready event starts. Even after Restored. And it was running before Ready for sure.

I could not find a better fix than adding a setTimeout, do you have better ideas here?

@joaomoreno
Copy link
Member

To repro, just run:

./scripts/code.sh --open-url code-oss://vscode.git/clone

It should show a native confirmation dialog. It currently doesn't.

@joaomoreno joaomoreno modified the milestones: July 2019, August 2019 Jul 31, 2019
@joaomoreno joaomoreno removed the help wanted Issues identified as good community contribution opportunities label Jul 31, 2019
@bpasero
Copy link
Member

bpasero commented Aug 5, 2019

@joaomoreno as discussed, the difference from before is that services are no longer created on startup unless they are needed. A combination of this service and an early workbench contribution could work.

@aeschli aeschli added the verified Verification succeeded label Aug 28, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug linux Issues with VS Code on Linux verified Verification succeeded windows VS Code on Windows issues workbench-os-integration Native OS integration issues
Projects
None yet
Development

No branches or pull requests

8 participants