Fix handling for source generated documents in LSIF#59891
Fix handling for source generated documents in LSIF#59891jasonmalinowski merged 2 commits intodotnet:mainfrom
Conversation
We correctly handled them in the core generation code, but failed as we were actually serializing out JSON. Fixes dotnet#59692
| // TODO: when we move to .NET Core, is there a way to reduce allocations here? | ||
| contentBase64Encoded = Convert.ToBase64String(Encoding.UTF8.GetBytes(text.ToString())); | ||
|
|
||
| uri = "source-generated://" + syntaxTree.FilePath.Replace('\\', '/'); |
There was a problem hiding this comment.
I think it would be good to land on a consistent URI scheme for source generated documents
For example currently LSP does this - https://github.com/dotnet/roslyn/blob/main/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs#L33
There was a problem hiding this comment.
@dibarbet, ah right my commit message of "we should figure a standard" didn't get updated here. 😄
The bigger thing we need is not just the prefix, but also what the other parts of the URI are as well, since we also need something to differentiate different projects/targets. Did you do anything for that?
There was a problem hiding this comment.
Not yet - the general idea would be to encode them as parameters.
Perhaps its OK to wait to unify until we start actually doing the virtual docs support. But maybe at least for now we should use the same prefix
There was a problem hiding this comment.
I chose source-generated rather than just "generated" to keep the scheme more explicit, since we may also have other schemes like metadata-as-source or source-link or something else and those need to go to different LSP handlers or something.
This scheme is arbitrary for now and just gives LSIF consuming tools a better idea where these files come from. Once we standardize a proper form for these for LSP, we should update this to match.
8bd3b32 to
64b5510
Compare
We correctly handled them in the core generation code, but failed as we were actually serializing out JSON.
Fixes #59692