-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Only qualify the first symbol in Quick Info #27946
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,5 +55,13 @@ public enum SymbolDisplayMiscellaneousOptions | |
/// the special question mark syntax. | ||
/// </summary> | ||
ExpandNullable = 1 << 5, | ||
|
||
/// <summary> | ||
/// Suppresses minimization of the first visited symbol. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have to admit, it's not clear to me what "first visited symbol" means in this case as an external consumer of the API. The visitor is an implementation detail of this API, no? It seems the term shouldn't appear here then. |
||
/// </summary> | ||
/// <remarks> | ||
/// Has no effect outside <see cref="ISymbol.ToMinimalDisplayString"/>. | ||
/// </remarks> | ||
QualifyFirstSymbol = 1 << 6, | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1864,10 +1864,10 @@ End Class | |
|
||
CompilationUtils.AssertTheseDiagnostics(compilation, | ||
<expected> | ||
BC30508: 'B' cannot expose type 'A.B(Of T).C' in class 'A' through class 'B'. | ||
BC30508: 'B' cannot expose type 'A.B(Of T As A.B(Of T).C).C' in class 'A' through class 'B'. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this change expected? The resulting error message is harder to read. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please see my comment above explaining why this happens and a possible alternative. I can go either way but wasn't sure about the broader impact of changing a predefined format. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After discussing this offline, I think we should consider changing the VB diagnostic format to report |
||
Private Class B(Of T As B(Of T).C) | ||
~~~~~~~~~ | ||
BC30508: 'D' cannot expose type 'A.B(Of T).C' in class 'A' through class 'B'. | ||
BC30508: 'D' cannot expose type 'A.B(Of T As A.B(Of T).C).C' in class 'A' through class 'B'. | ||
Public Sub D(Of S As C)() | ||
~ | ||
</expected>) | ||
|
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.
Why the forcing to .None?
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.
The kind is already being printed as part of the nested type. We don't want to include the type kind for the containing type or you could end up with a display like this: