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

Report document symbols in a tree view manner #34

Open
DoctorKrolic opened this issue Jun 25, 2023 · 7 comments
Open

Report document symbols in a tree view manner #34

DoctorKrolic opened this issue Jun 25, 2023 · 7 comments

Comments

@DoctorKrolic
Copy link
Collaborator

As of now, document symbols are reported as flat array, e.g. for this .csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <LangVersion>preview</LangVersion>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.7.0-1.final" />
    <PackageReference Include="Microsoft.EntityFrameworkCore" Version="7.0.7" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="7.0.7" />
  </ItemGroup>

</Project>

... reported symbols look like this:
Code_0QHbtD15dA

However, given the tree-like nature of xml and the fact, that LSP protocol supports tree-like symbol structure, it would make a lot more sence to report symbols as following:

Microsoft.NET.Sdk
-- PropertyGroup
---- OutputType
---- TargetFramework
---- ImplicitUsings
---- Nullable
---- LangVersion
---- AllowUnsafeBlocks
-- ItemGroup
---- PackageReference
---- PackageReference
---- PackageReference

Here not only tree structure is implemented, but also this tree represents document structure in a better way by including syntax elements like PropertyGroup or ItemGroup into it

@tintoy
Copy link
Owner

tintoy commented Jun 25, 2023

Interesting - I suspect that LSP has evolved significantly since the extension was first written; I'll have a look at the latest protocol definitions and see what would be involved 🙂

Thanks for the feedback!

@tintoy
Copy link
Owner

tintoy commented Jun 26, 2023

LSP spec:

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_documentSymbol

Note: client-supplied value for DocumentSymbolClientCapabilities. hierarchicalDocumentSymbolSupport has to be true before we can enable this behaviour.

@tintoy
Copy link
Owner

tintoy commented Jun 26, 2023

It may be slightly tricky to decide whether a given symbol should represent the XML-level property or property-group (MSBuild construction-level) or the logical one (MSBuild evaluation-level).

If it's really for document structure then I think we may have to follow the XML rather than evaluated properties but I'm open to alternatives 🙂

@tintoy
Copy link
Owner

tintoy commented Jun 27, 2023

Looking into this a bit further we may run into a limitation of the version of the OmniSharp LSP libraries we're using. Our current version is very old (and doesn't know anything about newer client/server capabilities such as symbol-trees) and I believe their language server hosting API has changed significantly since then.

So as part of this work we will need to bite the bullet and migrate to their current LSP implementation 🙂

@DoctorKrolic
Copy link
Collaborator Author

Microsoft has Common Language Server Protocol Framework (CLaSP), which seems to be a better alternative for developing language servers. It has been used internally for different products and, for instance, is behind razor LSP in VS/VS Code and roslyn LSP powering C# extension in C# Dev Kit. As of right now it isn't publicly available yet (at least it is not recommended for use), but it seems like it will be there soon. If OmniSharp APIs has changed so dramatically, maybe it makes sense to wait until CLaSP is available and move to it. Especially considering that, as I already mentioned, C# extension, that was previously powered by OmniSharp now uses roslyn LSP built on top of CLaSP, making future of OmniSharp APIs unclear.

@tintoy
Copy link
Owner

tintoy commented Jun 27, 2023

Interesting - I vaguely remember hearing about that a while back but didn't realise they were going to release their own framework; I suppose it may depend on the relative ergonomics of the 2 frameworks then. I'll try to have a look at them both (if possible) over the weekend and see how different they feel.

@tintoy
Copy link
Owner

tintoy commented Jun 27, 2023

Hmm - even at a glance the new CLaSP API seems nicer than the old OmniSharp API that I remember (although I haven't seen what the newer OmniSharp API looks like yet either). Will have a proper look at both over the weekend 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants