[LSP] Respect settings passed in by LSP in AutoInsertHandler #51916
Conversation
allisonchou
added
Area-IDE
LSP
allisonchou
requested review from
davidwengier,
NTaylorMullen,
CyrusNajmabadi and
dibarbet
| if (document == null) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var service = document.GetRequiredLanguageService<IDocumentationCommentSnippetService>(); | ||
|
|
||
| // We should use the options passed in by LSP instead of the document's options. | ||
| var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
are the options passed in by LSP correct? Like for example if you have an editor config setting for tabs vs spaces. Does LSP read that? Similarly for tools options settings.
There was a problem hiding this comment.
From testing with Razor, it correctly recognizes changes in Tools->Option settings. I'm not sure about editorconfig, will test shortly.
Regardless, I think we need this change since Razor files won't be correctly formatted without it, namely because we need to use Razor's Tools->Options settings instead of C#'s.
There was a problem hiding this comment.
It does indeed seem to support editorconfig as well (at least in C#, I think I recall editorconfig currently being unsupported for Razor files).
There was a problem hiding this comment.
Ok, I think it's probably fine to make this change, just one more question
Regardless, I think we need this change since Razor files won't be correctly formatted without it, namely because we need to use Razor's Tools->Options settings instead of C#'s.
Ah, and I guess with the dynamic file info they provide they don't have a way to set those options on the generated document?
There was a problem hiding this comment.
Ah, and I guess with the dynamic file info they provide they don't have a way to set those options on the generated document?
Even if that were the case I wouldn't imagine we'd want to. Given the platform is the owner of settings it should be the one informing you on what settings to use. Out of curiosity does on type formatting or document formatting have this same gap?
There was a problem hiding this comment.
Out of curiosity does on type formatting or document formatting have this same gap?
From a glance, this does seem to be the case. I can address this in a follow up PR.
ghost
added this to the 
Today, LSP passes in tabs/spaces settings as part of the OnAutoInsert request. However, we do not respect these settings and instead use the document's options. We should respect the LSP settings.
This will partially fix some of the tabs vs. spaces issues seen in Razor.