-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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 opening files through URL handling (fixes #4883) #15320
Conversation
@KattMingMing, thanks for your PR! By analyzing the history of the files in this pull request, we identified @egamma, @bpasero and @jrieken to be potential reviewers. |
Hi @KattMingMing, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
@KattMingMing, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
Fixes #4883 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intentions are good, and it appears at a first glance to just work. But there might be a better way to do this.
Right now, the URLService will send the onOpenURL
event to the last focused Window. In the case of opening a file, that might not be the ideal scenario. Think about having two windows, each with a workspace open: A and B. While A is the last focused one, you can open a vscode:file/B/foo.js
URL; ideally, the file would open in the window of workspace B, and not in A as it would occur given this PR.
For that reason, it might be better to react to the onOpenURL
event in the main process, where we control all the windows.
I suggest to do it in the WindowsService and just call the open
method in this other guy. That area is currently a heavy reconstruction site but I think you'll do fine.
fyi - we have an |
@jrieken That's already too late, since that service is scoped to an EditorService, which is scoped to a specific Window. |
@joaomoreno Great, thanks for the feedback! |
5871153
to
7d586b8
Compare
7d586b8
to
29fa9bb
Compare
@joaomoreno I removed the implementation from the EditorService and moved the listener and implementation inside of the WindowsService. The opening behavior is now the same as running |
Good job! I just cleaned up a little bit more, added a disposables list and removed the method declaration in the service interface, we want this to be private within the service itself. 🍻 Thanks! |
@joaomoreno Awesome, thanks for the update! 🍻 |
…cast) This PR was merged into the 4.2-dev branch. Discussion ---------- [FrameworkBundle] Add vscode editor to ide config | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#10424 <!-- required for new features --> <!-- Write a short README entry for your feature/bugfix here (replace this comment block.) This will help people understand your PR and can be used as a start of the Doc PR. Additionally: - Bug fixes must be submitted against the lowest branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the master branch. --> Support for uri handler (added [here](microsoft/vscode#15320)) in vscode editor. Commits ------- 05935d8 Add vscode editor to ide config
No description provided.