-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Improve and unify debug views of dictionaries. #92534
Conversation
The change alows generic and non-generic dictionaries to be displayed in the same way: with separate columns for keys and values with an ability to expand each column. Fixes dotnet#88736
Tagging subscribers to this area: @dotnet/area-system-collections Issue DetailsThe change allows generic and non-generic dictionaries to be displayed in the same way: with separate columns for keys and values with an ability to expand each column. Currently non-generic dictionaries are displayed using two columns but without an option to expand keys or values: Generic dictionaries are displayed as ordered lists of key/value pairs with a drill-down functionality: The PR unifies the way both kinds of dictionaries are displayed by a debugger. Dictionaries are shown using two-column format (which IMO better represents a dictionary) and both keys and values can be expanded: Fixes #88736
|
src/libraries/System.Private.CoreLib/src/System/Collections/Generic/IDictionaryDebugView.cs
Outdated
Show resolved
Hide resolved
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.
Awesome, thanks! We should investigate and check if other included dictionary types need the same treatment (including the ones in System.Collections.Immutable
and System.Collections.Frozen
).
@arturek this seems to be causing test failures in CI, could you take a look? |
@eiriktsarpalis I'll fix the tests. I thought they are unrelated but now I can see the reason they are failing. |
Included non-generic dictionaries in tests
Any luck getting the tests fixed? Thanks. |
* Included more types that implement IDictionary in the DebuggerView tests. * Improved the testing code to support classes with attributes declared by their base classes. * Fixed .Net Framework 4.8 build error by removing a dependency on the record c# feature. * Fixed tests remaining tests (outside of the System.Collections subset)
Still seeing a number of failing tests, e.g.
|
@eiriktsarpalis: The tests you mentioned failed because I'll look at it tomorrow. |
.Net Framwork does not support recent improvements in the way the debugger displays a dictionary. Depending on the framwork used, the debugger view of generic dictionaries and ListDictionaryInternal are different.
@eiriktsarpalis |
src/libraries/Common/tests/System/Collections/DebugView.Tests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Collections.NonGeneric/tests/SortedListTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Diagnostics/DebuggerAttributes.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.ObjectModel/tests/ReadOnlyDictionary/ReadOnlyDictionaryTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Collections/Generic/DebugViewDictionaryItem.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Collections/Generic/IDictionaryDebugView.cs
Outdated
Show resolved
Hide resolved
* mostly code style changes * restored a rest for an empty HashSet. * fixed testing of the generic SortedList.
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.
Almost forgot, but would it be possible to include this change in immutable collections as well? If you prefer to do this in a future PR that's also fine.
src/libraries/System.Private.CoreLib/src/System/Collections/Generic/DebugViewDictionaryItem.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Collections/Hashtable.cs
Outdated
Show resolved
Hide resolved
Renamed an internal method to match its new behavior and removed unnecessary init accessors.
It's on my list. I'd prefer to do it in a separate PR just to close this phase and keep scope of this PR smaller. |
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!
The change allows generic and non-generic dictionaries to be displayed in the same way: with separate columns for keys and values with an ability to expand each column.
Currently non-generic dictionaries are displayed using two columns but without an option to expand keys or values:
Generic dictionaries are displayed as ordered lists of key/value pairs with a drill-down functionality:
The PR unifies the way both kinds of dictionaries are displayed by a debugger. Dictionaries are shown using two-column format (which IMO better represents a dictionary) and both keys and values can be expanded:
Fixes #88736