tintoy / msbuild-project-tools-server

Language server for MSBuild intellisense (including PackageReference completion).
MIT License
59 stars 16 forks source link

Report document symbols in a tree view manner #34

Open DoctorKrolic opened 1 year ago

DoctorKrolic commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 πŸ™‚