Add outlining support to LsifGenerator#49197
Conversation
src/Features/LanguageServer/Protocol/Handler/FoldingRanges/FoldingRangesHandler.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Existing code moved here.
src/Features/Lsif/Generator/Microsoft.CodeAnalysis.LanguageServerIndexFormat.Generator.csproj
Outdated
Show resolved
Hide resolved
| ""id"": 7, | ||
| ""type"": ""edge"", | ||
| ""label"": ""textDocument/foldingRange"" | ||
| }, |
There was a problem hiding this comment.
| /// </remarks> | ||
| private static Id<Graph.LsifDocument> GenerateForDocument( | ||
| SemanticModel semanticModel, | ||
| private static async Task<Id<Graph.LsifDocument>> GenerateForDocumentAsync( |
There was a problem hiding this comment.
(note other comments about intentional lack of async and workspace types here...)
jasonmalinowski
left a comment
There was a problem hiding this comment.
So this generally looks good other than the use of Project/Document and async in the generator. That goes in the opposite direction of what I was trying to do to allow this to also run as an analyzer, down the road. @CyrusNajmabadi did you make further progress on that or have some preferences?
This change refactors and moves the BlockStructure service and providers to the shared Workspace utilities layer. The core APIs are now based off SyntaxTree instead of Document to allow the service to be used from an analyzer context, which is required for dotnet#49197.
src/Features/LanguageServer/Protocol/Handler/FoldingRanges/FoldingRangesHelper.cs
Outdated
Show resolved
Hide resolved
…dingRangesHelper.cs
src/Features/LanguageServer/Protocol/Handler/FoldingRanges/FoldingRangesHelper.cs
Outdated
Show resolved
Hide resolved
|
@jasonmalinowski @CyrusNajmabadi this is ready for review now, with much smaller set of changes to review. |
| return GetFoldingRanges(blockStructure, text); | ||
| } | ||
|
|
||
| public static FoldingRange[] GetFoldingRanges( |
There was a problem hiding this comment.
This method is called by LSIF generator.
| lsifJsonWriter.Write(new Event(Event.EventKind.End, documentVertex.GetId(), idFactory)); | ||
|
|
||
| // Write the folding ranges for the document. | ||
| var foldingRanges = FoldingRangesHandler.GetFoldingRanges(syntaxTree, languageServices, options, isMetadataAsSource: false, CancellationToken.None); |
There was a problem hiding this comment.
Out of curiosity, we're passing in an OptionSet to this -- which folding range providers used options in some way?
This code is fine -- what else are you going to do? -- but it's an interesting thing to think about both from the LSIF perspective but also LSP perspective of whether we need to compute things and include extra information that lets the consumer/client know how to interpret it given user options at that point in time.
There was a problem hiding this comment.
@jasonmalinowski I know that the block structure service itself is using these options: https://github.com/dotnet/roslyn/blob/master/src/Features/Core/Portable/Structure/BlockStructureOptions.cs. I did not check specific options used by the providers.
|
|
|
Hello @mavasani! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
No description provided.