diagnose generating overload duplicates - review#4
Conversation
…ad-duplicates # Conflicts: # ChaosGenerators/source/TemplateGenerator.cs
FramePerfection
left a comment
FramePerfection
left a comment
There was a problem hiding this comment.
This is effectively the split I had in mind myself, so thank you for executing it.
You may remove the unnecessary linebreaks before merging if you wish, without a need for a re-review.
There was a problem hiding this comment.
.NET 8 introduces EqualityComparer.Create that may be used to replace this class.
Whether this should happen seems mostly like a matter of taste or convenience right now, but I have the feeling that this class may be of use in a wider context in the future.
There was a problem hiding this comment.
The generator is no .Net 8, but a .Net Standard 2.0 library.
This function is only available in .Net 8 and .Net 9.
| HashSet<TemplateArgument[]> generatedOverloads | ||
| = new HashSet<TemplateArgument[]>(OverloadComparer.instance); |
There was a problem hiding this comment.
The linebreak here is now superfluous, as the new shortened array type doesn't uselessly deplete the character count of the line like the previous tuple.
No need to change that now, though - superfluous linebreaks are another issue.
| string ReplaceTemplateArgs( | ||
| string text, | ||
| ((Regex templateParameterRegex, string templateParameterName), ITypeSymbol templateArgument)[] templateArgs | ||
| TemplateArgument[] templateArgs | ||
| ) |
There was a problem hiding this comment.
Same as above (presumably, idk how GitHub actually deals with these comments...), those linebreaks don't need to exist with the much shorter type identifier.
I guess #1 will undergo some sort of cleanup before merging it into |
2 participants
requested changes for #2