-
Notifications
You must be signed in to change notification settings - Fork 133
Make sure tuple protocol name does not ignore unknown parameter types #206
Conversation
return sb.ToString(); | ||
} | ||
|
||
private string GetParameterString(AnalysisValue[] sets) { |
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.
you can reuse the same StringBuilder
here.
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.
Good point
return "?"; | ||
} | ||
var sb = new StringBuilder(); | ||
if (sets.Length > 1) { |
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.
This can be simplified:
If (sets.Length == 1) {
sb.Append(sets[0] is IHasQualifiedName qn ? qn.FullyQualifiedName : sets[0].ShortDescription)
} else {
// remaining code
}
} | ||
for (var i = 0; i < sets.Length; i++) { | ||
if (i > 0) { | ||
sb.Append(", "); |
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.
", "
or ","
?
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.
", " more readable
// Enumerate manually since SelectMany drops empty/unknown values | ||
var sb = new StringBuilder("tuple["); | ||
for (var i = 0; i < _values.Length; i++) { | ||
if (i > 0) { |
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.
We can have an extension method for this:
https://github.com/Microsoft/RTVS/blob/master/src/Common/Core/Impl/Extensions/StringBuilderExtensions.cs
Make sure tuple protocol name does not ignore unknown parameter types
Additional fix for #173 since 'SelectMany' drops empty sequences and tuples lose arguments of unknown type making them look like they have fewer arguments.