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

Fix vscode.commands.executeCommand() documentation #80758

Closed
wants to merge 2 commits into from
Closed

Fix vscode.commands.executeCommand() documentation #80758

wants to merge 2 commits into from

Conversation

joelspadin
Copy link

As noted in #80338, when executing a command that is contributed by an extension
that is not yet loaded results in serializing all arguments. Only the types
allowed to be passed to editor commands and JSON-serializable objects are
guaranteed to survive being passed to another extension.

As noted in #80338, when executing a command that is contributed by an extension
that is not yet loaded results in serializing all arguments. Only the types
allowed to be passed to editor commands and JSON-serializable objects are
guaranteed to survive being passed to another extension.
* * *Note 2:* When executing commands that have been contributed by other
* extensions, any JSON-serializable objects are additionally allowed.
* * *Note 3:* There are no restrictions when executing commands that have
* been contributed by the extension that is executing them.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your comment here #80758 (comment) is much better because that's how it is. Generally, there are no restrictions when calling a command that's contributed by another extension unless that command is used to activate the extension.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also hint towards Extension#isActive and Extension#activate here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Updated with something that I hope succinctly describes the rules.

@jrieken jrieken added this to the September 2019 milestone Sep 12, 2019
@jrieken jrieken added api polish Cleanup and polish issue labels Sep 12, 2019
Reworded the documentation to explicitly say that there are no restrictions if
the extension is active and some restrictions if it isn't.
@jrieken
Copy link
Member

jrieken commented Sep 13, 2019

The working is now better I think overall the problem shouldn't be there to begin with. I have pushed a change for #80338 that makes this doc change not needed. Still, thanks for drilling into this.

@jrieken jrieken closed this Sep 13, 2019
@joelspadin
Copy link
Author

Great! I like that solution much better.

@joelspadin joelspadin deleted the documentation-fix branch September 13, 2019 16:32
@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
api polish Cleanup and polish issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants