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

Unexpected return type of vscode.executeFormatDocumentProvider and vscode.executeDocumentSymbolProvider #769

Closed
lukehoban opened this issue Nov 28, 2015 · 9 comments
Assignees
Labels
feature-request Request for new features or functionality
Milestone

Comments

@lukehoban
Copy link
Member

It's nice that the implementations of FormatDocumentProvider and DocumentSymbolProvider can be used via executeCommand. However, the return types are undocumented and subtely different from what the implementation side produces.

  1. Is this intentional?
  2. Is it considered a supported scenario for extensions to use executeCommand on these commands? Or should they factor out their implementation and use the implementation code directly?

For FormatDocumentProvider for example, I provide a TextEdit[] which is:

[
 {
  "_range": {
   "_start": {
    "_line": 2,
    "_character": 7
   },
   "_end": {
    "_line": 4,
    "_character": 4
   }
  },
  "_newText": ""
 },
 {
  "_range": {
   "_start": {
    "_line": 4,
    "_character": 14
   },
   "_end": {
    "_line": 5,
    "_character": 1
   }
  },
  "_newText": ""
 }
]

but executeCommand("vscode.executeFormatDocumentProvider", ...) receives this as:

[
 {
  "text": "",
  "range": {
   "startLineNumber": 3,
   "startColumn": 8,
   "endLineNumber": 5,
   "endColumn": 5
  }
 },
 {
  "text": "",
  "range": {
   "startLineNumber": 5,
   "startColumn": 15,
   "endLineNumber": 6,
   "endColumn": 2
  }
 }
]

The combination of completely different shape plus the 1-based numbering makes this data difficult to work with.

Similarly different results are returned from others similar commands.

@ironcladlou
Copy link
Contributor

Another example is executeDocumentSymbolProvider which returns an object similar to a SymbolInformation.

@alexdima
Copy link
Member

@lukehoban @ironcladlou I know @jrieken is working on a solution for this.

Extensions should never get to see the 1-based format (that is the format that reaches the main process).

For now, please don't write code depending / working around these wrong return values as we plan to fix them.

@ironcladlou
Copy link
Contributor

@alexandrudima

For now, please don't write code depending / working around these wrong return values as we plan to fix them.

Are you implying these commands should be "private" or unstable? What is the supported API to get the symbols for a document by URI?

@lukehoban
Copy link
Member Author

@ironcladlou I think the supported way would be to factor out the symbol collection code from the DocumentSymbolProvider and then call into that shared implementation from both the symbol provider and any places that want to use the symbol information. In the vscode-go extension in particular, we already do something like this for the formatting code, and could do something similar for the symbol code.

@ironcladlou
Copy link
Contributor

@lukehoban

I think the supported way would be to factor out the symbol collection code from the DocumentSymbolProvider and then call into that shared implementation from both the symbol provider and any places that want to use the symbol information. In the vscode-go extension in particular, we already do something like this for the formatting code, and could do something similar for the symbol code.

My understanding is that multiple providers can be registered for a given document selector; Code then merges the results to feed the rest of the IDE (via private APIs?) Seems to me that extensions would benefit from exported API access to the merged/processed symbols the IDE already collected from all extensions. I also wouldn't be surprised if the processed provider results were also cached internally, etc for performance, stuff we wouldn't want to duplicate.

Coupling the vscode-go functionality to its own single provider's implementation would probably work, but seems a little less than ideal based on the above.

Thoughts?

@lukehoban
Copy link
Member Author

Good point.

I was more commenting on what should be done for vscode-go, where in fact it is likely more correct to factor out the symbol logic than to rely on potentially getting results from other providers.

@jrieken jrieken added the feature-request Request for new features or functionality label Dec 1, 2015
@jrieken jrieken added this to the Dec 2015 milestone Dec 1, 2015
@jrieken
Copy link
Member

jrieken commented Dec 3, 2015

I think there are valid use-cases for both scenarios, using commands and using extension api. For instance, the command-approach you can use to make UI extensions, like hover-information showing in the status bar: 1. listen to selection changes, 2. run command vscode.executeHoverProvider, 3. update status bar.

Since by using the commands you don't decide which provider actually produces the data and since the commands and data types are defined by us you might need more. Think of a special code action that requires a syntax tree etc. To enable that an extension should export special APIs that are then consumed by dependent extensions.

Wrt the commands we have pushed many changes to master which should make them accept and return the correct data types. Still, we have no figured out yet what the best way of documenting them is. We will track that in #913

@lukehoban
Copy link
Member Author

@jrieken BTW - per microsoft/vscode-go#176, it appears that all calls to executeCommand("vscode.executeFormatDocumentProvider", uri) are now failing with Running the contributed command:'vscode.executeDocumentSymbolProvider' failed. in vscode 0.10.6.

For the particular problem that was causing in the Go extension, we've changed the code to avoid using this command and just call Go's symbol provider directly, which is likely more correct in this case.

@jrieken
Copy link
Member

jrieken commented Jan 25, 2016

@lukehoban We now check that calling the 'API commands' happens with the spec'd arguments, that would be [uri, options] and [uri, range, options]

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

4 participants