Skip to content

Conversation

tokarzkj
Copy link
Contributor

@tokarzkj tokarzkj commented Nov 19, 2019

Users will now be offered refactoring options for collection initializers. The implementation was modelled after the AbstractSeparatedSyntaxList implementation. This applies to visual basic and c#.

There are 3 choices for users:

  1. Wrap every element
// Before
var example = new[] { 1, 2 };

// After
var example = new[] 
{
    1,
    2
};
  1. Unwrap every element
// Before
var example = new[] 
{
    1,
    2
};

// After
var example = new[] { 1, 2 };
  1. Wrap long
// Before
var example = new[] { 1, 2 };

// After
var example = new[]
{
    1, 2
};

Resolves #37428

The basic class is in place for visual basic to have the appropriate
wrapping refactoring. The wrapping is mostly correct and just needs
a small tweak for the closing brace to be on a line by itself.
The closing brace is calculated based on what kind of syntax we
are dealing with. If it is a DeclarationKind then we can just align
it based on the span start. If it is a return statement we need to
add an extra tab for spacing because there is no declarationkind for
returns and we passed our starting point.
- After wrapping if you tried to get refactorings again it was throwing
an exception. Calculating the distance between first node before the {
fixed the issue.

- The unwrap refactoring wasn't moving the opening { to the previous line
These tests confirm that one when one of the refactoring offerings has
been choosen if the user asks for more refactorings they are offered
the other two options to rewrite the initializer.
Also, remove the old comments that were copied over from the other
wrapping code computer implementations
Resolve the conflicts in FeatureResources.resx
@tokarzkj tokarzkj marked this pull request as ready for review November 19, 2019 01:53
@tokarzkj tokarzkj requested a review from a team as a code owner November 19, 2019 01:53
where TListSyntax : SyntaxNode
where TListItemSyntax : SyntaxNode
{
protected override string Indent_all_items => FeaturesResources.Indent_all_arguments;
Copy link
Member

Choose a reason for hiding this comment

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

"arguments"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, I thought I changed that. I was going to look at removing these sub labels too unless you think they would be better left in. But, as of now each refactoring only has one option and that seems pointless.


namespace Microsoft.CodeAnalysis.CSharp.Wrapping.InitializerExpression
{
internal partial class CSharpInitializerExpression : AbstractCSharpInitializerExpressionWrapper<InitializerExpressionSyntax, ExpressionSyntax>
Copy link
Member

Choose a reason for hiding this comment

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

CSharpInitializerExpressionWrapper

@CyrusNajmabadi
Copy link
Member

@jasonmalinowski i believe this community PR is ready for team review. thanks!

@jasonmalinowski
Copy link
Member

@CyrusNajmabadi, yep, it's on my list along with seemingly everybody else's. Should get to it in a day or two.

tokarzkj and others added 3 commits January 28, 2020 18:44
…omputer.cs

Co-Authored-By: CyrusNajmabadi <cyrus.najmabadi@gmail.com>
Co-Authored-By: CyrusNajmabadi <cyrus.najmabadi@gmail.com>
Co-Authored-By: CyrusNajmabadi <cyrus.najmabadi@gmail.com>
@CyrusNajmabadi
Copy link
Member

Thanks @jasonmalinowski !

Base automatically changed from master to main March 3, 2021 23:52
@sharwell
Copy link
Contributor

sharwell commented Feb 8, 2022

📝 Note that this feature needs to coordinate with the code style from #54859. Since this PR is much older, it seems like it should merge first and then we update #54859 to make the necessary changes there.

@sharwell
Copy link
Contributor

sharwell commented Feb 8, 2022

I'm working on resolving the merge conflicts...

@sharwell sharwell enabled auto-merge February 8, 2022 21:07
@CyrusNajmabadi CyrusNajmabadi force-pushed the initializer-expression-wrapper branch from 425c27b to cb26d8f Compare February 8, 2022 23:25
// MethodName(
// int a, int b, int c, int d, int e, int f, int g, int h, int i, int j)
unwrapActions.Add(await GetUnwrapAllCodeActionAsync(parentTitle, WrappingStyle.WrapFirst_IndentRest).ConfigureAwait(false));
if (this.Wrapper.Supports_UnwrapGroup_WrapFirst_IndentRest)
Copy link
Member

Choose a reason for hiding this comment

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

i recommend reviewing with whitespace off.

@CyrusNajmabadi CyrusNajmabadi merged commit ef902b7 into dotnet:main Feb 11, 2022
@ghost ghost added this to the Next milestone Feb 11, 2022
@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

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. Feature Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrap array initializer expressions.

6 participants