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

Do not solely make Visual Diagnostics available on debugger being attached #23490

Merged
merged 5 commits into from
Jul 23, 2024

Conversation

chabiss
Copy link
Contributor

@chabiss chabiss commented Jul 8, 2024

Description of Change

This changes ensure that Visual Diagnostics doesn't rely on the debugger being attached, like the rest of the supported platforms (WPF, UWP, WinUI). Other platform relies on ENABLE_XAML_DIAGNOSTICS_SOURCE_INFO to be set by the Launch Provider in VS to enable Full Visual Diagnostics with Source Information. For backward compatibility, MAUI will still continue to check for the debugger being attached, but will also check the presence of the environment.

Issues Fixed

Task 2078603: Remove MAUI's VisualDiagnostics coupling on the Debugger

@chabiss chabiss requested a review from StephaneDelcroix July 8, 2024 23:51
@chabiss chabiss requested a review from a team as a code owner July 8, 2024 23:51
@chabiss chabiss requested a review from tj-devel709 July 8, 2024 23:51
@rmarinho
Copy link
Member

rmarinho commented Jul 9, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@@ -13,16 +13,19 @@ namespace Microsoft.Maui
public static class VisualDiagnostics
{
static ConditionalWeakTable<object, SourceInfo> sourceInfos = new ConditionalWeakTable<object, SourceInfo>();
static Lazy<bool> isVisualDiagnosticsEnvVarSet = new Lazy<bool>(() => Environment.GetEnvironmentVariable("ENABLE_XAML_DIAGNOSTICS_SOURCE_INFO") is { } value && value == "1");

static public bool IsEnabled => DebuggerHelper.DebuggerIsAttached || isVisualDiagnosticsEnvVarSet.Value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be a public variable? Since it's not settable and not used outside of this repo I'm not sure if it's needed to be accessible.

Copy link
Contributor Author

@chabiss chabiss Jul 10, 2024

Choose a reason for hiding this comment

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

Good point! It should be internalvisible to Microsoft.Maui.Controls

@chabiss chabiss force-pushed the dev/chabiss/azdo_2078603 branch from db21032 to 5eaf90a Compare July 15, 2024 20:34
Copy link
Contributor

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

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

This doesn't guarantee all the information will be available. some diagnostics are generated by the runtime XAML parser, and not by XAMLC

@StephaneDelcroix StephaneDelcroix merged commit 4ef069f into main Jul 23, 2024
97 checks passed
@StephaneDelcroix StephaneDelcroix deleted the dev/chabiss/azdo_2078603 branch July 23, 2024 05:27
@chabiss
Copy link
Contributor Author

chabiss commented Jul 23, 2024

@StephaneDelcroix Indeed, I'm attempting to discern whether there's a difference from how things were prior to this PR. Are there still Debugger checks in the runtime XAML Parser that I might have overlooked or need to modify?

@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-8.0.80 and removed fixed-in-net9.0-nightly This may be available in a nightly release! labels Aug 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-8.0.80 fixed-in-net9.0-nightly This may be available in a nightly release!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants