Skip to content
This repository has been archived by the owner on Oct 4, 2021. It is now read-only.

[Debugger] Add logpoints to the icon margin #4588

Closed
wants to merge 1 commit into from
Closed

Conversation

iainx
Copy link
Contributor

@iainx iainx commented Apr 17, 2018

A log point is a breakpoint that just prints a message and continues. Add
support for them to the icon margin.

Requires mono/debugger-libs#182

@iainx iainx requested a review from slluis as a code owner April 17, 2018 10:24
A log point is a breakpoint that just prints a message and continues. Add
support for them to the icon margin.
@mhutch
Copy link
Member

mhutch commented Apr 17, 2018

How are these different from tracepoints?

@mhutch
Copy link
Member

mhutch commented Apr 17, 2018

Some context:
microsoft/vscode#45128
microsoft/vscode-node-debug2#163

It looks like VS Code is calling tracepoints "logpoints". However, since VS also calls them tracepoints, I think we should stick with the existing name for now.

I can see value in having a "new tracepoint" command (like VS) that opens up the breakpoint editor already configured as a tracepoint. But I'm not sure about "toggle tracepoint" - we can't simply toggle a tracepoint on, we need to ask for an expression, which means showing a UI, and toggle off is already covered by the breakpoint toggle command.

Note also that in VS the breakpoint menu is shown in the editor, not the margin - because breakpoints can have a column offset too when there are multiple statements on one line.

I wonder whether we should investigate making it easier to change breakpoint properties after creation with something like the floating toolbar and inline access breakpoint properties like VS: https://blogs.msdn.microsoft.com/devops/2014/10/06/new-breakpoint-configuration-experience-in-visual-studio-2015.

@Therzok
Copy link
Contributor

Therzok commented Apr 18, 2018

See #4518

@mhutch
Copy link
Member

mhutch commented Apr 18, 2018

Yup. Definitely agree making this functionality more discoverable and usable is good, just think it needs some design work.

Copy link
Contributor

@Therzok Therzok left a comment

Choose a reason for hiding this comment

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

Unit tests please

Copy link
Member

@slluis slluis left a comment

Choose a reason for hiding this comment

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

I agree with @mhutch. I don't think we need to introduce a new logpoint concept, and it makes more sense to have an Add command rather than toggle.

@jstedfast
Copy link
Member

Based on the comments, it sounds like this should be closed? Or?

@iainx iainx closed this May 23, 2019
@iainx
Copy link
Contributor Author

iainx commented May 23, 2019

Yeah, it was a quick proof of concept, so closed

@jstedfast jstedfast deleted the logpoints branch May 23, 2019 13:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants