-
-
Notifications
You must be signed in to change notification settings - Fork 213
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
Code generator improvements #934
Code generator improvements #934
Conversation
@StefH would you mind reviewing/merging this pr? Thanks in advance. |
will do later today |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small question... See comments
@@ -537,4 +555,85 @@ private static string FormatArray(JArray jArray, int ind) | |||
|
|||
return $"new [] {{ {string.Join(", ", items)} }}"; | |||
} | |||
|
|||
private static readonly HashSet<string> csharp_keywords = new HashSet<string>(new[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this to the top of the file?
(And maybe instead of using this list, you can maybe use:
https://github.com/StefH/ProxyInterfaceSourceGenerator/blob/main/src/ProxyInterfaceSourceGenerator/Extensions/SymbolExtensions.cs#L35
) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using SyntaxFacts.GetKeywordKind
requires reference to Microsoft.CodeAnalysis.CSharp
.
I can extract the whole logic responsible for generating anonymous object to a separate class, what fo you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, maybe a new internal static class, placed in Util
folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
…efinition to a separate class
No description provided.