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

Add helper in EnumSingleTypeConverter to get enum names-array #17785

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fflaten
Copy link
Contributor

@fflaten fflaten commented Jul 27, 2022

PR Summary

Adds a helper in EnumSingleTypeConverter to get enum names-array.
Updates switch statement completer to use the new helper instead of string joining + splitting.

PR Context

Related #17750 (comment)

There's another occurrence in CompletionCompleters.cs which is replaced as part of the PR above.

PR Checklist

@fflaten fflaten changed the title Use helper in EnumSingleTypeConverter to get enum names-array Add helper in EnumSingleTypeConverter to get enum names-array Jul 27, 2022
@fflaten
Copy link
Contributor Author

fflaten commented Jul 27, 2022

Feels like random failures. Rerun?

@fflaten
Copy link
Contributor Author

fflaten commented Jul 28, 2022

Just noticed that #17684 plan to replace this code. This PR can be closed if the other one is accepted.

string enumString = LanguagePrimitives.EnumSingleTypeConverter.EnumValues(typeof(SwitchFlags));
string separator = CultureInfo.CurrentUICulture.TextInfo.ListSeparator;
string[] enumArray = enumString.Split(separator, StringSplitOptions.RemoveEmptyEntries);
string[] enumArray = LanguagePrimitives.EnumSingleTypeConverter.GetEnumNames(typeof(SwitchFlags));
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is one more place where we can use the helper if I remember correctly.

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 is the one you mentioned, not covered by #17750 . I don't see other references besides log/exception-messages where they want this comma-separated string.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please fix both in the PR.

Copy link
Contributor Author

@fflaten fflaten Jul 29, 2022

Choose a reason for hiding this comment

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

? No need to update code I've suggested to remove in #17750

Even the line changed might be unnecessary due to #17684

@ghost ghost added the Review - Needed The PR is being reviewed label Aug 5, 2022
@ghost
Copy link

ghost commented Aug 5, 2022

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review - Needed The PR is being reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants