Pass per-document options when organizing imports or usings #17347
Conversation
| @@ -12,28 +14,31 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Organizing | |||
| { | |||
| public class OrganizeUsingsTests | |||
| { | |||
| protected async Task CheckAsync(string initial, string final, bool specialCaseSystem, CSharpParseOptions options = null) | |||
| protected async Task CheckAsync(string initial, string final, bool placeSystemNamespaceFirst = false, CSharpParseOptions options = null) | |||
There was a problem hiding this comment.
i hate that this is optional.
There was a problem hiding this comment.
I can make it required. Just saw a lot of tests passing in random true/false values even when it didn't affect the test. false is the default in VS. It seemed reasonable that only tests that were explicitly testing this behavior should change it. should parse options be non-optional as well?
There was a problem hiding this comment.
i'm ok with this being optional now :)
| { | ||
| using (var workspace = await TestWorkspace.CreateCSharpAsync(initial)) | ||
| { | ||
| var document = workspace.CurrentSolution.GetDocument(workspace.Documents.First().Id); | ||
| var newRoot = await (await OrganizeImportsService.OrganizeImportsAsync(document, specialCaseSystem)).GetSyntaxRootAsync(); | ||
| var workspaceOptions = workspace.Options; | ||
| var newOptionSet = workspaceOptions.WithChangedOption(new OptionKey(GenerationOptions.PlaceSystemNamespaceFirst, document.Project.Language), placeSystemNamespaceFirst); |
There was a problem hiding this comment.
inline both these variables.
| cancellationToken As CancellationToken) As Task(Of Document) Implements IOrganizeImportsService.OrganizeImportsAsync | ||
| Dim root = Await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(False) | ||
| Dim options = Await document.GetOptionsAsync(cancellationToken).ConfigureAwait(False) | ||
| Dim placeSystemNamespaceFirst = options.GetOption(GenerationOptions.PlaceSystemNamespaceFirst, document.Project.Language) |
There was a problem hiding this comment.
isn't passing hte language unnecessary? ti's known in the DocumentOptions that you got from document.GetOptionsAsync.
There was a problem hiding this comment.
The option is a PerLanguageOption because it can be set differently for C# or VB if you don't use editorconfig
There was a problem hiding this comment.
right. but options.GetOption, when passed a PerLanguageOption, does not need a language passed in as well. Right?
| @@ -4,16 +4,18 @@ Imports System.Threading | |||
| Imports Microsoft.CodeAnalysis.Host.Mef | |||
| Imports Microsoft.CodeAnalysis.OrganizeImports | |||
| Imports System.Composition | |||
| Imports Microsoft.CodeAnalysis.Editing | |||
fixes #17342