Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Debugger display of dictionary types and DebuggerDisplayAttribute.Name #88736

Closed
JamesNK opened this issue Jul 12, 2023 · 4 comments · Fixed by #92534
Closed

Debugger display of dictionary types and DebuggerDisplayAttribute.Name #88736

JamesNK opened this issue Jul 12, 2023 · 4 comments · Fixed by #92534
Labels
area-System.Collections enhancement Product code improvement that does NOT require public API changes/additions
Milestone
Future

Comments

@JamesNK
Copy link
Member

JamesNK commented Jul 12, 2023 •
edited
Loading

DebuggerDisplayAttribute has Name and Type properties. We never use these.

A place Name is useful is collection items. It can be used to display a custom name instead of an index in a debugger window.

Before:

image

After:

image

This seems like an improvement to me. It's easier to see what the keys and values are.

I was wondering why IDictionaryDebugView and other dictionary type proxies don't use this feature. Right now the key and value are squashed together in the debugger, and a meaningless index is displayed as the name.
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 12, 2023
@ghost
Copy link

ghost commented Jul 12, 2023

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

DebuggerDisplayAttribute has Name and Type properties. We never use these.

A place Name is useful is collection items. It can be used to display a custom name instead of an index in a debugger window.

Before:

image

After:

image

This seems like an improvement to me.

I was wondering why IDictionaryDebugView and other dictionary type proxies don't use this feature. Right now the key and value are squashed together in the debugger, and a meaningless index is displayed as the name.

Author: JamesNK
Assignees: -
Labels:

untriaged, area-Debugger-mono
Milestone: -

@ghost
Copy link

ghost commented Jul 13, 2023

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

DebuggerDisplayAttribute has Name and Type properties. We never use these.

A place Name is useful is collection items. It can be used to display a custom name instead of an index in a debugger window.

Before:

image

After:

image

This seems like an improvement to me. It's easier to see what the keys and values are.

I was wondering why IDictionaryDebugView and other dictionary type proxies don't use this feature. Right now the key and value are squashed together in the debugger, and a meaningless index is displayed as the name.

Author: JamesNK
Assignees: -
Labels:

area-System.Collections, untriaged
Milestone: -

@JamesNK
Copy link
Member Author

JamesNK commented Jul 13, 2023

@jaredpar Curious if you have any insight here or historical context.

@eiriktsarpalis eiriktsarpalis added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Jul 13, 2023
@eiriktsarpalis eiriktsarpalis modified the milestones: 9.0.0, Future Jul 13, 2023
@jaredpar
Copy link
Member

Don't really have any historical context on this specific decision.

I will say that in general the rules of DebuggerDisplay and the power when combined with DebuggerTypeProxy and DebuggerBrowsable wasn't fully appreciated by everyone when it first came out. That combined with various bugs in the EE, perf issues evaluating so many new expressions, and actual bugs how developers structured meant they weren't as utilized as they could have been. It wasn't until probably VS 2010 until I think the entire system was in a fully formed state.

It's quite possible the original author didn't realize these type of tricks were possible, or tried and gave up due to the mentioned issues and no one has revisited it since then.

@stephentoub stephentoub mentioned this issue Sep 2, 2023
Closed
arturek added a commit to arturek/runtime that referenced this issue Sep 23, 2023
@arturek
Improves and unifies debug views of dictionaries.
e7963bc 
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
@arturek arturek mentioned this issue Sep 23, 2023
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 23, 2023
eiriktsarpalis pushed a commit that referenced this issue Nov 2, 2023
@arturek
Improve and unify debug views of dictionaries. (#92534)
34bf55c 
* Improves and unifies debug views of dictionaries.

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 #88736

* Fix the DebuggerView tests of dictionaries

Included non-generic dictionaries in tests

* Fix more DebuggerView tests

* 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)

* Fix DebugView.Tests build errors on .Net Framework

* Update DebugView tests to expect different outcomes on .Net Framework

.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.

* Applied suggested changes and fixes

* mostly code style changes
* restored a rest for an empty HashSet.
* fixed testing of the generic SortedList.

* Minor improvents

Renamed an internal method to match its new behavior and removed unnecessary init accessors.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 2, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-System.Collections enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Milestone
Future
Development

Successfully merging a pull request may close this issue.

Improve and unify debug views of dictionaries. arturek/runtime
3 participants
@jaredpar @JamesNK @eiriktsarpalis