-
Notifications
You must be signed in to change notification settings - Fork 219
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 support for LogPoints in OpenDebugAD7 #1013
Conversation
This PR includes a TracepointManager for OpenDebugAD7 which will keep track of LogMessages included in BreakpointRequests. LogMessages can include expression inside of { } for the engine to evaluate.
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.
See comments
@WardenGnaw Please add an example of input/output in your original comments so we can get an idea on what the output looks like in VS Code |
Thanks for the example. Is there a way to hide that breakpoint hit message in the middle of that message? |
That will be part of the |
@@ -54,7 55,7 @@ internal sealed class DebuggerTelemetry | |||
public const string TelemetryVisualizerFileUsed = "VisualizerFileUsed"; | |||
public const string TelemetrySourceFileMappings = "SourceFileMappings"; | |||
public const string TelemetryMIMode = "MIMode"; | |||
public const string TelemetryStackFrameId = TelemetryExecuteInConsole ".ExecuteInConsole"; | |||
public const string TelemetryStackFrameId = TelemetryExecuteInConsole ".StackFrameId"; |
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.
note: remember to classify this after its been shipped.
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.
Same for the tracepoint one.
} | ||
catch (InvalidTracepointException e) | ||
{ | ||
DebuggerTelemetry.ReportError(DebuggerTelemetry.TelemetryTracepointEventName, e.Message); |
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.
will this have a meaningful message? I don't remember you setting the message when you were throwing.
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.
The only time this exception is thrown is when we cant find a matching '}' for interpolation.
@@ -94,10 93,9 @@ private int GetInterpolatedLogMessage(string logMessage, IDebugThread2 pThread, | |||
else | |||
{ | |||
string toInterpolate = keyValuePair.Value; | |||
hr = InterpolateVariable(toInterpolate.Substring(1, toInterpolate.Length - 2), topFrame[0].m_pFrame, radix, out value); | |||
if (hr < 0) | |||
if (InterpolateVariable(toInterpolate.Substring(1, toInterpolate.Length - 2), topFrame[0].m_pFrame, radix, out value) < 0) |
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.
why not make this return value the HR? and then ...?
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.
I did not want to stop the log message from interpolating half way through. This allows it to completly interpolate the expressions and indicate an error has occured for the ones that failed (if there are multiple)
Add support for LogPoints in OpenDebugAD7 (#1013)
This PR includes a TracepointManager for OpenDebugAD7 which will
keep track of LogMessages included in BreakpointRequests.
LogMessages can include expression inside of { } for the
engine to evaluate.
TODO: $PID since all we have is an AD_PROCESS_ID.