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 opening files through URL handling (fixes #4883) #15320

Merged
merged 1 commit into from
Nov 14, 2016

Conversation

KattMingMing
Copy link
Contributor

No description provided.

@mention-bot
Copy link

@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.

@msftclas
Copy link

Hi @KattMingMing, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@msftclas
Copy link

@KattMingMing, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@joaomoreno
Copy link
Member

Fixes #4883

@joaomoreno joaomoreno added this to the November 2016 milestone Nov 11, 2016
Copy link
Member

@joaomoreno joaomoreno left a 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.

@jrieken
Copy link
Member

jrieken commented Nov 11, 2016

fyi - we have an OpenerService which should do this

@joaomoreno
Copy link
Member

@jrieken That's already too late, since that service is scoped to an EditorService, which is scoped to a specific Window.

@KattMingMing
Copy link
Contributor Author

@joaomoreno Great, thanks for the feedback!

@KattMingMing
Copy link
Contributor Author

@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 code -g -r /path/to/file. Thanks again for taking the time to review

@bpasero bpasero removed their assignment Nov 12, 2016
@joaomoreno joaomoreno merged commit 29fa9bb into microsoft:master Nov 14, 2016
@joaomoreno
Copy link
Member

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!

@KattMingMing
Copy link
Contributor Author

@joaomoreno Awesome, thanks for the update! 🍻

@KattMingMing KattMingMing deleted the mk/issue_4883 branch November 14, 2016 17:29
nicolas-grekas added a commit to symfony/symfony that referenced this pull request Oct 2, 2018
…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
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants