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 support for LogPoints in OpenDebugAD7 #1013

Merged
merged 8 commits into from
Jun 26, 2020
Merged

Conversation

WardenGnaw
Copy link
Member

@WardenGnaw WardenGnaw commented Jun 19, 2020

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.

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.
@WardenGnaw WardenGnaw self-assigned this Jun 19, 2020
@gregg-miskelly
Copy link
Member

                            breakpointRequest is AD7BreakPointRequest ad7BPRequest)

Is there a reason to maintain your own collection instead of just storing this in the breakpoint request object?


Refers to: src/OpenDebugAD7/AD7DebugSession.cs:1604 in df983e1. [](commit_id = df983e1, deletion_comment = False)

Copy link
Member

@gregg-miskelly gregg-miskelly left a comment

Choose a reason for hiding this comment

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

See comments

src/OpenDebugAD7/Tracepoint.cs Outdated Show resolved Hide resolved
src/OpenDebugAD7/Tracepoint.cs Outdated Show resolved Hide resolved
@pieandcakes
Copy link
Collaborator

@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

@WardenGnaw
Copy link
Member Author

Example usage:
image

@pieandcakes
Copy link
Collaborator

Thanks for the example. Is there a way to hide that breakpoint hit message in the middle of that message?

@WardenGnaw
Copy link
Member Author

That will be part of the logging.threadEvents flag work.

@@ -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";
Copy link
Collaborator

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.

Copy link
Collaborator

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);
Copy link
Collaborator

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.

Copy link
Member Author

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.

@WardenGnaw WardenGnaw merged commit 185e133 into master Jun 26, 2020
@@ -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)
Copy link
Collaborator

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

Copy link
Member Author

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)

WardenGnaw added a commit that referenced this pull request Jun 30, 2020
Add support for LogPoints in OpenDebugAD7 (#1013)
@WardenGnaw WardenGnaw deleted the dev/waan/SupportLogPoints branch August 7, 2020 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants