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

debug focus automatically switches to newly created debug session #128132

Closed
weinand opened this issue Jul 7, 2021 · 11 comments
Closed

debug focus automatically switches to newly created debug session #128132

weinand opened this issue Jul 7, 2021 · 11 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded

Comments

@weinand
Copy link
Contributor

weinand commented Jul 7, 2021

Steps:

  • unpack and open attached Cluster.zip project in VS Code
  • set breakpoint in test.js line 15
  • start debugging by running the "Cluster" launch config
  • after hitting breakpoint press Continue button on debug toolbar

Observe: a new debug session starts and the corresponding node in the CALL STACK gets selected. As a consequence the debug focus switches to the new session and the debug toolbar no longer shows the "Continue" button. In order to continue stepping, a stopped debug session must be selected in the CALL STACK view.
This happens for every new child process of the cluster...

2021-07-07_16-05-26 (1)

A new debug session should never change the selected session in the CALL STACK view (at least if there is already a stopped debug session.

@weinand weinand added bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues labels Jul 7, 2021
@weinand weinand added this to the July 2021 milestone Jul 7, 2021
@isidorn
Copy link
Contributor

isidorn commented Jul 28, 2021

I checked last two stables (1.58 and 1.57) and we were already behaving like this.
Whenever there is a new session it would steal focus, even if there is already a session that is stopped and focused.
I would prefer not to change this as part of endgame -> August. Especially since the js-debug sort of relies on this that the child session which comes after the parent session would take the focus away.

also fyi @connor4312 to get his thoughts

@isidorn isidorn modified the milestones: July 2021, August 2021 Jul 28, 2021
@connor4312
Copy link
Member

Yea I think if a debug session is paused, focus should not be automatically moved away

@weinand
Copy link
Contributor Author

weinand commented Jul 28, 2021

If this is not a regression, I'm fine with not fixing this for July. But we should fix this at the beginning of the August cycle.

@gjsjohnmurray
Copy link
Contributor

Maybe #121898 is related.

@isidorn
Copy link
Contributor

isidorn commented Jul 29, 2021

I pushed a similar fix for #128400
However the problem with this issue is the following:

So the only way to solve this that I see is to auto focus only after a timeout. Which Visual Studio does not do.
Let me know your thoughts.

@baybal
Copy link

baybal commented Jul 29, 2021

Given this is a very rare opportunity to catch people who work on VScode JS debugger, I have a small offtopic question

How does the debugger know the line number on which console.log was called, and how a wrapper for it can pass it along to the real console.log while preserving object inspectability, formatting, colours, etc?

It's important for logger packages like https://github.com/winstonjs/winston

@connor4312
Copy link
Member

connor4312 commented Jul 29, 2021

@baybal you can file issues here or on js-debug and I usually respond within 24h 😉 The debugger knows the line console.log is called on since the call stack reported by V8. The first non-skipped location in the stack will be shown, which you can adjust using skipFiles patterns.

@weinand
Copy link
Contributor Author

weinand commented Aug 4, 2021

@isidorn above you said:

once a user presses continue we do not know that the Session 1 will be stopped again in a couple of ms. I have basically mentioned this problem here #125144 (comment)

I do not understand how your statement is related to the issue at hand:

my ask was: A new debug session should never change the selected session in the CALL STACK view because the new session is running and there is nothing that needs to be shown by focusing on it. Only if the new session stops, we should focus on it.

So there is no other paused process/thread to which we can/should pass the focus (as was asked in #125144).

@connor4312 connor4312 added the verified Verification succeeded label Aug 25, 2021
@weinand weinand added verification-found Issue verification failed and removed verified Verification succeeded labels Aug 30, 2021
@weinand weinand reopened this Aug 30, 2021
@weinand
Copy link
Contributor Author

weinand commented Aug 30, 2021

I cannot verify this.

Please see:

2021-08-30_12-47-29 (1)

@isidorn
Copy link
Contributor

isidorn commented Aug 30, 2021

The fix I did, did not cover the case when all sessions are siblings.

Before we would always focus on a new child session if the parent session was focused. This does not really make sense in general, and would only make sense for js-debug. I have removed this behaviour and this indeed fixes the issue.
So in theory now when a user starts a js-debug session which does not hit any breakpoints the parent session will remain focused until the child gets stopped. I think this makes sense in general.

I suggest to do this for the September milestone, since we have always behaved like this and to give some time to catch potential regressions. I have pushed a fix for this to main, if somebody would like to have this as part of the release please let me know and I can create a PR against the release branch.

fyi @roblourens

@isidorn isidorn modified the milestones: August 2021, September 2021 Aug 30, 2021
@weinand
Copy link
Contributor Author

weinand commented Aug 31, 2021

@isidorn this works now for my Cluster.zip sample.

@weinand weinand added verified Verification succeeded and removed verification-found Issue verification failed labels Aug 31, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants
@weinand @isidorn @connor4312 @baybal @gjsjohnmurray and others