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

add TreeItem API/provider #253

Closed
WebFreak001 opened this issue Jun 9, 2017 · 12 comments
Closed

add TreeItem API/provider #253

WebFreak001 opened this issue Jun 9, 2017 · 12 comments

Comments

@WebFreak001
Copy link

TreeItems got added to 1.13
https://code.visualstudio.com/docs/extensionAPI/vscode-api#TreeItem
https://code.visualstudio.com/docs/extensionAPI/extension-points#_contributesviews

@ljw1004
Copy link
Contributor

ljw1004 commented Jun 12, 2017

Just to note, I think the existing "documentSymbols" is almost exactly the right API to provide an "Outline View" as used in Atom, IntelliJ, Eclipse, VisualStudio. The only thing it needs is one small addition: it currently emits containerName which is enough to reconstruct the outline view in most cases, but it should also emit containerLocation so that outline view can be reconstructed in all cases.

Anyway, I don't actually understand what "TreeItem" is for or what it looks like, and didn't get enough understanding from those two links. But I wanted to flag that if it's a tree for outline view, then I think we should tread carefully.

@daviwil
Copy link
Contributor

daviwil commented Jun 12, 2017

It's for any kind of tree that might be useful in the Explorer view: alternative file explorer, project navigation, convenient command pane, etc. General purpose.

@daviwil
Copy link
Contributor

daviwil commented Jun 12, 2017

However, I should clarify that I don't necessarily think this should be a core part of the LSP. Since this is a VS Code UI feature and not necessarily something that's generally applicable to editors, not sure if it makes sense as a first-class member of the protocol.

That's just my opinion, though, I'd be fine if there was first-class support for tree views since I'm going to implement in my language server either way :)

@egamma
Copy link
Member

egamma commented Jun 12, 2017

fyi @jrieken

@WebFreak001
Copy link
Author

Yeah it is basically a vscode only feature but I think it would actually be quite useful in any generic editor. And as servers & clients both don't need to support all features I don't think it would hurt to add it. It isn't a big deal if it doesn't get added as we can add it with custom functions. Usage example for this feature is for example a nodejs dependency viewer like in the visual studio plugin or a dependency viewer (also like in visual studio), just this could be used and customized by language server authors.

@mickaelistria
Copy link

I fully support previous comment from @daviwil .

The TreeItem from VSCode, as I understand it, doesn't seem at all related to programming languages but more an advanced widget in VSCode for rendering trees (like other UI toolkits may already have). It's more specific to VSCode's UI layer than a feature of an IDE related to a programming language. I don't think it makes sense in the LSP.

There are already multiple solutions to build a nice and complete tree from the documentSymbols: using containers, using range inclusion... There is no need for a new concept to provide tree, the LSP already covers that properly.

I don't think it would hurt to add it.

Adding stuff that's not consistent is making the API a bit messier and then harder to evolve and maintain. We should avoid being to greedy and keep focused on finding operations or notifications that makes sense for an IDE to request to a language.

@jrieken
Copy link
Member

jrieken commented Jun 13, 2017

Just to note, I think the existing "documentSymbols" is almost exactly the right API to provide an "Outline View"

Having something like an outline view is tracked here: microsoft/vscode#5605. Indeed, there are already extensions using the tree API and api command to get to an outline view. I think that is a valid stepping-stone until we are tackling that in the provider-spirit. By that I mean, will spec a new DocumentSymbolProvider that supports a tree of symbols. The UI implementation is then up to the editors (as we have done it already with things like reference search or completion) and will most likely be a tree but also something like a break-crump view atop of a file will be possible when having the right data-api spec'd.

The TreeItem from VSCode, as I understand it, doesn't seem at all related to programming languages but more an advanced widget in VSCode for rendering trees

That is correct. The TreeItem represents what is shown in the UI, whereas the tree data provider is the source of items that are to be rendered (in a tree structure)

@mickaelistria
Copy link

So it seems like we all agree it's a vscode request ;) Should we close this issue?
@jrieken Eclipse IDE already support a hierarchical/tree-based outline populated with document symbols from LSP. See https://git.eclipse.org/r/plugins/gitiles/lsp4e/lsp4e/+/0.2.0/org.eclipse.lsp4e/src/org/eclipse/lsp4e/outline/SymbolsModel.java#34 for an example of how to use the ranges returned by the documentSymbols to create a tree-like structure. And breadcrumbs are a great idea, I've tracked it here for addition in Eclipse IDE https://bugs.eclipse.org/bugs/show_bug.cgi?id=518182

@jrieken
Copy link
Member

jrieken commented Jun 13, 2017

Should we close this issue?

Yeah, I'd say it will be better to track this in the vscode-repo. Ideas is to branch out from the catch-all-issue, with define provider and data model, define bread-crumbs ui, define tree ui

@ljw1004
Copy link
Contributor

ljw1004 commented Jun 13, 2017

@mickaelistria I'm looking at the eclipse implementation of reconstructing outline view from documentSymbols.

This implementation trusts that the "location.range" returned from documentSymbols will be the range of the entire symbol, from the start of line 10 right up to the end of line 40 in the code below.

10: class Foo
20: {
30:    ...
40: }

Is that really correct? My understanding was that documentSymbols returns the range purely of the symbol identifier itself, purely the range of the Foo on line 10. I guess @bruno-medeiros had the same doubt: #132

@mickaelistria
Copy link

Not sure it's correct conforming to the LSP spec, but at least, it works for VSCode CSS, JSon and HTML language servers. But I agree there are some good reasons to doubt about it ;)

@dbaeumer
Copy link
Member

I agree that we should close the issue and will do so.

The design goal of the LSP was to provide language 'smartness' that can be shared between tools not to implement UI that can be shared between tools. I think there is some space for shared descriptive UI as well but I would argue that this should be a different protocol than LSP.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants