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

Unify user experience with disparate 'generate constructor' features #76328

Merged
merged 12 commits into from
Dec 9, 2024

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented Dec 7, 2024

Fixes #34206

Merges 'generate base-delegating constructors' and 'generate constructors from members' into one experience so that users don't see 'generate constructor XXX' show up in disparate locations in teh lightbulb

Looks like this:

image

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner December 7, 2024 22:39
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 7, 2024
@@ -1514,7 +1514,8 @@ protected B(int x)
{
}
}
""");
""",
index: 1);
Copy link
Member Author

Choose a reason for hiding this comment

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

intentional. because code actions were merged, the 0th is now the one in this group that uses the dialog to pick the membes, while the 1nth is the one that creates a delegating constructor.

<data name="Generate_constructor" xml:space="preserve">
<value>Generate constructor...</value>
<data name="Generate_constructor_from_members" xml:space="preserve">
<value>Generate constructor from members...</value>
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed this string to be clearer about what selecting it will do. Note '...' is our consistent suffix for indicating the user will get a dialog to do the work with. In this case, the new title is indicating that invokign this will give them the dialog taht allows them to pick the members they want the constructor to initialize.

internal abstract partial class AbstractGenerateConstructorFromMembersCodeRefactoringProvider
using static GenerateFromMembersHelpers;

internal abstract partial class AbstractGenerateConstructorsCodeRefactoringProvider
Copy link
Member Author

Choose a reason for hiding this comment

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

renamed this type to indicate it's for any sort of 'generate constructor' refactoring expereince. not just generating based on members.

// Finally, offer to generate default constructors that delegate to the base type if any are missing.
var defaultConstructorService = document.GetRequiredLanguageService<IGenerateDefaultConstructorsService>();
var defaultConstructorActions = await defaultConstructorService.GenerateDefaultConstructorsAsync(
document, textSpan, forRefactoring: true, cancellationToken).ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

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

this was all the other refactoring provider did. just merged it into this shared refactoring provider.

var service = document.GetRequiredLanguageService<IGenerateDefaultConstructorsService>();
var actions = await service.GenerateDefaultConstructorsAsync(
document, textSpan, forRefactoring: true, cancellationToken).ConfigureAwait(false);
context.RegisterRefactorings(actions);
Copy link
Member Author

Choose a reason for hiding this comment

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

this was the seperate refactoringg provider. all it did was call into this service and register these refactorings. this is now moved into the single refactoring provider that does all gen-constructor work.

@@ -18,13 +17,9 @@

namespace Microsoft.CodeAnalysis.GenerateFromMembers;

internal abstract partial class AbstractGenerateFromMembersCodeRefactoringProvider : CodeRefactoringProvider
internal static class GenerateFromMembersHelpers
Copy link
Member Author

Choose a reason for hiding this comment

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

changed from an abstract base class to a static helper class. this was just full of static helpers and no state. so we didn't need inheritance.

@CyrusNajmabadi
Copy link
Member Author

@ToddGrun @JoeRobich ptal

Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

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

:shipit:

@CyrusNajmabadi CyrusNajmabadi merged commit 1bf248f into dotnet:main Dec 9, 2024
25 checks passed
@CyrusNajmabadi CyrusNajmabadi deleted the genConstructor branch December 9, 2024 20:06
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate constructor/Generate default constructor should be grouped together
2 participants