Skip to content

Initial work to move indentation services down to codestyle layer (step 8/N)#60451

Merged
CyrusNajmabadi merged 16 commits intodotnet:mainfrom
CyrusNajmabadi:indentationDown8
Mar 29, 2022
Merged

Initial work to move indentation services down to codestyle layer (step 8/N)#60451
CyrusNajmabadi merged 16 commits intodotnet:mainfrom
CyrusNajmabadi:indentationDown8

Conversation

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

Followup to https://github.com/dotnet/roslyn/pull/60445

This PR moves the remainder of the IndentationService down. The only thing remaining is moving existing that use this down to codestyle as well.

I'm doing these PRs in multiple parts to make things simpler to review. Every time i try to tackle the general problem, it explodes into a ton of changes.

The reason for this change overall is that we have fixers taht need to know indentation information. For example, the 'convert to file-scoped-namespace (and reverse)' fixes have to know indentation information so it can make the right fixes. Not having indentation info be available at thsi level means that the fixes are constrained to the VS host.

@CyrusNajmabadi CyrusNajmabadi requested a review from mavasani March 29, 2022 06:58
var formattingRuleProvider = workspace.Services.GetService<IHostDependentFormattingRuleFactoryService>();

var rules = formattingRuleProvider.CreateRule(document, position).Concat(Formatter.GetDefaultFormattingRules(document));
var rules = ImmutableArray.Create(formattingRuleProvider.CreateRule(document, position)).AddRange(Formatter.GetDefaultFormattingRules(document));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could the IHostDependentFormattingRuleFactoryService return an immutable array? or does it need to be ienumerable for some consumer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

teh host actually returns a single rule. but then we need to wrap that in an immutable array to be able to addrange after that. it's def not clean and i wish tehre was a better idiom for "i want to make an immutable array out of individual objects and streams of objects".

var syntaxFormatting = this.SyntaxFormatting;

#if CODE_STYLE
var tree = document.GetSyntaxTreeAsync(cancellationToken).WaitAndGetResult_CanCallOnBackground(cancellationToken);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to me would read better if there was just a GetTreeAndIndentation and GetIndentationRule method that kind of hid the codestyle messiness, but that might just be me

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. i don't love this. but i don't feel like there are really good alternaives here :(

public static string GetPreferredIndentation(this SyntaxToken token, Document document, CancellationToken cancellationToken)
{
#if CODE_STYLE
var sourceText = document.GetTextAsync(cancellationToken).WaitAndGetResult_CanCallOnBackground(cancellationToken);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the codestyle layer didn't use documents or am I totally off base

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not the codefixes layer, which can use documents (but only the public facing side of them).

@CyrusNajmabadi CyrusNajmabadi merged commit 82dd0f2 into dotnet:main Mar 29, 2022
@ghost ghost added this to the Next milestone Mar 29, 2022
@dibarbet dibarbet modified the milestones: Next, 17.3.P1 Apr 25, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the indentationDown8 branch May 3, 2022 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants