Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format list patterns #57568

Merged
merged 9 commits into from
Feb 18, 2022
Merged

Conversation

davidwengier
Copy link
Contributor

@davidwengier davidwengier commented Nov 4, 2021

Fixes #57244

Extract out a common base class from the CurlyBraceCompletionService so that parts can be shared with BracketBraceCompeltionService for list patterns, where we essentially want to format brackets as braces

Relates to test plan #51289

@davidwengier davidwengier requested review from a team as code owners November 4, 2021 04:58
@davidwengier davidwengier changed the base branch from main to features/list-patterns November 4, 2021 04:58

namespace Microsoft.CodeAnalysis.CSharp.BraceCompletion
{
internal abstract class AbstractCurlyBraceOrBracketCompletionService : AbstractBraceCompletionService
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This whole class is just a move from CurlyBraceCompletionService except where noted.

var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);

var startPoint = openingPoint;
var endPoint = AdjustFormattingEndPoint(text, root, startPoint, closingPoint);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code in CurlyBraceCompletionService.AdjustFormattingEndPoint was previously inline at this point

{
var originalRoot = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var closeBraceToken = originalRoot.FindToken(closingBraceEndPoint - 1);
Debug.Assert(IsValidClosingBraceToken(closeBraceToken));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This used to directly check for SyntaxKind.CloseBrace

@davidwengier
Copy link
Contributor Author

Ping @jcouv @sharwell for review

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

Done with review pass (iteration 7). Didn't review the existing code that we merely moved

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 9)

@jcouv jcouv self-assigned this Feb 18, 2022
@davidwengier davidwengier merged commit 6352f23 into dotnet:main Feb 18, 2022
@ghost ghost added this to the Next milestone Feb 18, 2022
@davidwengier davidwengier deleted the FormatListPatterns branch February 18, 2022 08:47
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P2 Mar 1, 2022
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.

Formatting of list-patterns on multiple lines is incorrect
3 participants