microsoft / vscode-go

An extension for VS Code which provides support for the Go language. We have moved to https://github.com/golang/vscode-go
Other
5.93k stars 645 forks source link

Feature Request: add option for Outline view to show package symbol tree rather than just for active editor #2793

Closed srinathh closed 4 years ago

srinathh commented 5 years ago

Currently, the outline view shows the symbol tree only in the active editor. Go's natural paradigm of development is however packages rather than files & in a package, it doesn't matter which file a particular function or struct is put in. When working on a large package having the Outline view show the natural package (or package main) symbol tree will make it easier to jump between functions rather than having to keep track mentally which function lives in which file.

ramya-rao-a commented 4 years ago

I am not the best judge when it comes to Go's natural paradigms, so looping in a few friends from the Go tools team :)

@stamblerre Any thoughts here? Please loop in others from the team if needed.

stamblerre commented 4 years ago

I'm not sure if symbols actually do make more sense in the scope of a package. I find them to be mostly useful for finding a symbol in a file, so I would imagine that people would want the ability to see both a file and a package view. I don't think VS Code's UI is capable of handling this behavior, or at least LSP doesn't have any way to do this. Still, this question might be useful to raise with the folks who work on LSP - there are a number of cases in Go where packages make more sense than files.

gertcuykens commented 4 years ago

I completely agree that it going to be hard to implement but I don't agree that symbols in a file is good enough. The only reason why people aren't complaining more is that go went mainstream waaaay before any go ide in a world were go tools were king. So we are used to work like Rob Pike and all his colleagues that have godoc open in a browser, some random text editor and a cmd line. If godoc was suddenly only capable to show symbols of one file only, the go gods would rain thunder and lightning and stop the world from rotating until godoc scope of package was restored. So what I am trying to say is as long as core features like a package outline aren't implemented most developers would gravitate to going back to go tools instead of using lsp.

srinathh commented 4 years ago

Glad this is getting discussion. In Go, files temd to have arbitrary collections of code and organizing namespace is package. For instance in the http package, some files contain lots of functions while some contain a single function. However all of them can see all others including the private functions under the package namespace.

When developing packages, I constantly find myself wanting to switch between functions/structures etc in other files within the package and having an option to see these in the outline would be useful and consistent with Go's paradigm.

I am pretty sure that Go's documentation tooling can serve up lists of package symbols and file location. I don't know if Visual Studio Code's outline view lets a file other than the active editor be specified

gertcuykens commented 4 years ago

Rebecca Stambler mentioned lsp doesn't have any way to do this. So first we need a gopls issue ticket addressing this. What worries me is that there isn't one already as far as I can tell? It worries me because it means there is a huge communication gap between golang developers and gopls developers. Do we need to take away godoc first before people actually start communicating what they really need from a ide?

stamblerre commented 4 years ago

To clarify, when I say that LSP doesn't support this I mean that the actual protocol doesn't have a way to express this functionality. See textDocument/documentSymbols, which operates per-file. Note that the Ranges specified in the result do not contain a URI, so there is no way to express a position in another file. This is something that can be raised with the LSP (https://github.com/microsoft/language-server-protocol), but it's not something that we can handle in gopls without modifications to the protocol.

gertcuykens commented 4 years ago

hmm I fail to see why this would be impossible in LSP considering multiple document request based on a extraction from go list -m all. I agree, not going to be straight forward but modifying the LSP protocol won't make it more easy right?

type DocumentUri = string;
interface Location {
    uri: DocumentUri;
    range: Range;
}
export interface SymbolInformation {
    /**
     * The name of this symbol.
     */
    name: string;

    /**
     * The kind of this symbol.
     */
    kind: SymbolKind;

    /**
     * Indicates if this symbol is deprecated.
     */
    deprecated?: boolean;

    /**
     * The location of this symbol. The location's range is used by a tool
     * to reveal the location in the editor. If the symbol is selected in the
     * tool the range's start information is used to position the cursor. So
     * the range usually spans more then the actual symbol's name and does
     * normally include things like visibility modifiers.
     *
     * The range doesn't have to denote a node range in the sense of a abstract
     * syntax tree. It can therefore not be used to re-construct a hierarchy of
     * the symbols.
     */
    location: Location;

    /**
     * The name of the symbol containing this symbol. This information is for
     * user interface purposes (e.g. to render a qualifier in the user interface
     * if necessary). It can't be used to re-infer a hierarchy for the document
     * symbols.
     */
    containerName?: string;
}
stamblerre commented 4 years ago

We return DocumentSymbol instead of SymbolInformation from textDocument/documentSymbols because it is a more detailed response type (allows for hierarchies, etc.). It was added in later versions of the protocol. That type contains a Range instead of a Location, which does not include the URI.

gertcuykens commented 4 years ago

Hmm and what about putting the location in detail? Do we have control over the lsp client to parse detail or is vscode itself the client and we can only modify the lsp server response?

/**
 * Represents programming constructs like variables, classes, interfaces etc. that appear in a document. Document symbols can be
 * hierarchical and they have two ranges: one that encloses its definition and one that points to its most interesting range,
 * e.g. the range of an identifier.
 */
export interface DocumentSymbol {

    /**
     * The name of this symbol. Will be displayed in the user interface and therefore must not be
     * an empty string or a string only consisting of white spaces.
     */
    name: string;

    /**
     * More detail for this symbol, e.g the signature of a function.
     */
    detail?: string;

    /**
     * The kind of this symbol.
     */
    kind: SymbolKind;

    /**
     * Indicates if this symbol is deprecated.
     */
    deprecated?: boolean;

    /**
     * The range enclosing this symbol not including leading/trailing whitespace but everything else
     * like comments. This information is typically used to determine if the clients cursor is
     * inside the symbol to reveal in the symbol in the UI.
     */
    range: Range;

    /**
     * The range that should be selected and revealed when this symbol is being picked, e.g the name of a function.
     * Must be contained by the `range`.
     */
    selectionRange: Range;

    /**
     * Children of this symbol, e.g. properties of a class.
     */
    children?: DocumentSymbol[];
}
gertcuykens commented 4 years ago

Found the ticket why they didn't include location, https://github.com/microsoft/language-server-protocol/issues/582 So basicly we need to go back to the gopls drawing board or comment on the LSP issue from more then a year ago that adresses this issue, explaining we are doing it the wrong way. Alternatively we introduce a option to strip out outline completely form vscode-go so other plugins can try to tackle it.

stamblerre commented 4 years ago

Just to clarify - we are not handling textDocument/documentSymbols incorrectly in gopls, and IMO, it entirely depends on a user's preference.

To quote https://github.com/microsoft/vscode-languageserver-node/pull/425#issuecomment-430560803:

DocumentSymbols are as the name implies scoped to a document and we shouldn't start changing this.

Different IDEs may have different behaviors, but in the context of LSP, we have the correct behavior. If you would like to open an issue with the protocol, I'd be happy to support the change, but as it stands, I don't think we can make any changes in gopls.

gertcuykens commented 4 years ago

Correct me if I am wrong. I make the assumtion that the active editor in vscode-go does one document request that basicly ends up here https://github.com/golang/tools/blob/master/internal/lsp/symbols.go Then from that response []protocol.DocumentSymbol most features, including outline, get extracted right? So even if a DocumentSymbol did had a uri field the result would be the same because the request is only made for one source.go file. There doesn't exist a package level request where you give it a package name and you get [][]protocol.DocumentSymbol So basically we need a protocol.Package instead of a uri in DocumentSymbol right?

interface Package {
    src: []Document;
}

interface Document {
    uri: DocumentUri;
    symbols: []DocumentSymbol;
}

https://github.com/microsoft/language-server-protocol/issues/878

stamblerre commented 4 years ago

Thank you for filing the upstream issue. All that's really needed on the gopls side is the ability to send back a URI for DocumentSymbol, since gopls knows which packages a file belongs to and could send back all of the symbols for the given file's package.

ramya-rao-a commented 4 years ago

@stamblerre The upstream issue for LSP https://github.com/microsoft/language-server-protocol/issues/878 has been closed. What are the next steps here?

stamblerre commented 4 years ago

I don't think we should stray from the LSP here. Workspace symbols combined with the document outline should be sufficient here, and if it's not, please open a new gopls issue: https://github.com/golang/go/issues/new.