Support opening files through URL handling (fixes #4883)#15320
Support opening files through URL handling (fixes #4883)#15320joaomoreno merged 1 commit intomicrosoft:masterfrom
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.
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! |
KattMingMing
force-pushed
the
mk/issue_4883
branch
from
5871153 to
7d586b8
Compare
KattMingMing
force-pushed
the
mk/issue_4883
branch
from
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
6 participants
No description provided.