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

Improve debugger experience #27017

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

pictos
Copy link
Contributor

@pictos pictos commented Jan 8, 2025

Description of Change

This PR adds the DebuggerDisplay attribute over a good amount of controls and, this is the first step into improving the debug experience of .NET MAUI
image

You can find more images in the related issue

Issues Fixed

Related to #27016

@pictos pictos requested a review from a team as a code owner January 8, 2025 23:10
@pictos pictos requested review from rmarinho and jsuarezruiz January 8, 2025 23:10
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Jan 8, 2025
Copy link
Contributor

Hey there @pictos! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@jfversluis
Copy link
Member

jfversluis commented Jan 9, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

jfversluis
jfversluis previously approved these changes Jan 9, 2025
Copy link
Member

@jfversluis jfversluis left a comment

Choose a reason for hiding this comment

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

2 minor things. But this little optimization makes me happy!

Thanks for putting this together Pedro!

src/Controls/src/Core/SearchBar/SearchBar.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/Application/Application.cs Outdated Show resolved Hide resolved
@jfversluis jfversluis added the area-architecture Issues with code structure, SDK structure, implementation details label Jan 9, 2025
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

How do things like commands look? Also, maybe we should also do events for Button. HasClicked = (Clicked is not null)


private protected override string GetDebbugerDisplay()
{
return $"Text = {Text}, Command = {Command}, " + base.GetDebbugerDisplay();
Copy link
Member

Choose a reason for hiding this comment

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

How does this look? The command will just be command, so not super useful? Maybe I am wrong tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would work like the BindingContext, here's an image to show how it will work

image

Co-authored-by: Gerald Versluis <[email protected]>
Copy link

Azure Pipelines successfully started running 3 pipeline(s).


private protected override string GetDebuggerDisplay()
{
return $"IsRunning = {IsRunning}, " + base.GetDebuggerDisplay();
Copy link
Contributor

Choose a reason for hiding this comment

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

Interpolating along with string concatenation ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would it cause any issue?

Copy link
Contributor

@MartyIX MartyIX Jan 10, 2025

Choose a reason for hiding this comment

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

would it cause any issue?

Typically one uses one or the other for consistency reasons, not both. I think one can do:

$"IsRunning = {IsRunning}, {(base.GetDebuggerDisplay())}";

Copy link
Contributor

Choose a reason for hiding this comment

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

String concatenation should never be use in modern c#

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-architecture Issues with code structure, SDK structure, implementation details community ✨ Community Contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants