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.

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

Add DebuggerDisplay for SearchValues #86559

Merged
merged 1 commit into from
May 22, 2023

Conversation

stephentoub
Copy link
Member

Before:
image

After:
image

@stephentoub stephentoub requested a review from MihaZupan May 21, 2023 18:43
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 21, 2023
@ghost ghost assigned stephentoub May 21, 2023
@stephentoub stephentoub added area-System.Runtime and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 21, 2023
@ghost
Copy link

ghost commented May 21, 2023

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

Issue Details

Before:
image

After:
image

Author: stephentoub
Assignees: stephentoub
Labels:

area-System.Runtime

Milestone: -

@EgorBo
Copy link
Member

EgorBo commented May 21, 2023

Just a general question: why this can't be just ToString() impl?

{
T[] values = GetValues();

string display = $"{GetType().Name}, Count={values.Length}";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using this in my testing, helps a ton when there are multiple generic variants.

Type implType = searchValuesInstance.GetType();
string impl = $"{implType.Name} [{string.Join(", ", implType.GenericTypeArguments.Select(t => t.Name))}]";

E.g.
SingleStringSearchValuesThreeChars`2 [ValueLength8OrLonger, CaseSensitive]

@MihaZupan
Copy link
Member

Overriding ToString seems reasonable as well

@stephentoub
Copy link
Member Author

This is effectively a collection. We don't override ToString on collections, and if we did, this wouldn't be the format chosen. The purpose of DebuggerDisplay is to raise for quick consumption the most relevant information for debugging purposes, not for runtime formatting. We should also feel free to frequently change a DebuggerDisplay rendering whereas doing so with ToString would not be desirable as code can take a dependency on it, it can be persisted in logs, test output, etc. This also needn't increase app size and can be trimmed as a debugger display.

@stephentoub stephentoub merged commit d2d51c8 into dotnet:main May 22, 2023
@stephentoub stephentoub deleted the searchvaluesdebuggerdisplay branch May 22, 2023 01:55
@ghost ghost locked as resolved and limited conversation to collaborators Jun 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants