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

Introduce compact flag to DebugSessionOptions #100852

Closed
isidorn opened this issue Jun 23, 2020 · 8 comments
Closed

Introduce compact flag to DebugSessionOptions #100852

isidorn opened this issue Jun 23, 2020 · 8 comments
Assignees
Labels
api-finalization debug Debug viewlet, configurations, breakpoints, adapter issues insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes on-testplan
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Jun 23, 2020

Refs: #99736

We are now compressing debug sessions in the call stack view when there is only one child session.
Since this behavior might not be ideal for all debug extension out of safety we would like to introduce a new flag noCompact to the DebugSessionOptions.

If the option is not specified would be the same as false and thus we would compact the sessions.

fyi @jrieken

@isidorn isidorn added debug Debug viewlet, configurations, breakpoints, adapter issues api-proposal labels Jun 23, 2020
@isidorn isidorn added this to the June 2020 milestone Jun 23, 2020
@isidorn isidorn self-assigned this Jun 23, 2020
@isidorn
Copy link
Contributor Author

isidorn commented Jun 23, 2020

Feedback from api call: it is fine to introduce the option as proposed and then next milestone we will see if debug extensions actually use it, and if nobody wants it we remove it.
Also the name should be compact not compress since that is how we call it in explorer settings.

@isidorn
Copy link
Contributor Author

isidorn commented Jun 25, 2020

I have introduced a noCompact flag to the DebugSessionOptions.
@weinand and @connor4312 feedback on name is welcome. I went with the compact since that is the user facing name of the explorer.compactFolders setting.

We wanted to make it negative, since the default that it is not specified should be the same as false.
Also aligns with noDebug.

@isidorn
Copy link
Contributor Author

isidorn commented Jun 25, 2020

Since we have added this api as proposed and created a test plan item, I am moving this out to on-deck for finalisation.
Name changes we can of course still do.

@isidorn
Copy link
Contributor Author

isidorn commented Jul 2, 2020

We have renamed this flag to be called compact, and to be opt-in. Thus by default we will not compact the session with it's parent.

@isidorn isidorn added the on-release-notes Issue/pull request mentioned in release notes label Jul 2, 2020
@isidorn isidorn modified the milestones: On Deck, August 2020 Aug 12, 2020
@isidorn
Copy link
Contributor Author

isidorn commented Aug 12, 2020

We plan to finalize this API this August milestone. Just in case there is more feedback before we finalize it.

fyi @connor4312

@connor4312
Copy link
Member

I've adopted this in js-debug, no real complaints. I had to change some behavior since certain interactions now always target the child, but these were my problems.

@isidorn isidorn changed the title Introduce incompressible flag to DebugSessionOptions Introduce compact flag to DebugSessionOptions Aug 18, 2020
@isidorn
Copy link
Contributor Author

isidorn commented Aug 19, 2020

The current JS Doc comment on the compact flag is

Controls if the debug session's parent session is shown in the CALL STACK view even if it has only a single child.
		 * By default, the debug session will never hide its parent.
		 * If compact is true, debug sessions with a single child are hidden in the CALL STACK view to make the tree more compact.

Feedback from API sync:

"We should make it more clear in the comment that the parent and the child session are actually rendered together since some actions (like stop) might be executed on the parent."

For now I will not change the comment. But @connor4312 and @weinand let me know what you think and if you would like me to change the comment.

@isidorn
Copy link
Contributor Author

isidorn commented Aug 19, 2020

I have finalized this API and have created this test plan item to cover the finalisation #104986

@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-finalization debug Debug viewlet, configurations, breakpoints, adapter issues insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes on-testplan
Projects
None yet
Development

No branches or pull requests

3 participants
@isidorn @connor4312 and others