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

Stack paging should not be implemented with a click #5849

Closed
joaomoreno opened this issue Apr 26, 2016 · 8 comments
Closed

Stack paging should not be implemented with a click #5849

joaomoreno opened this issue Apr 26, 2016 · 8 comments
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality *out-of-scope Posted issue is not in scope of VS Code
Milestone

Comments

@joaomoreno
Copy link
Member

joaomoreno commented Apr 26, 2016

Testing #4819

There are very good building blocks in our code for us to implement this properly:

Just talk to me for tips.

@joaomoreno joaomoreno added the feature-request Request for new features or functionality label Apr 26, 2016
@isidorn
Copy link
Contributor

isidorn commented Apr 26, 2016

We have explicitly decided on the explicit action, if you have concerns please comment here
#4792

Thanks for offering tips though :)

@isidorn isidorn closed this as completed Apr 26, 2016
@joaomoreno joaomoreno reopened this Apr 27, 2016
@joaomoreno
Copy link
Member Author

That explicit decision was taken based on the loading stack frames is expensive argument. What? The reason not to do lazy loading is because loading is slow? You are doing lazy loading, you're just implementing it with a very poor UI concept: an extra clickable list item.

The only thing that you are missing to implement nice paging, like we do with the extensions, is the total number of stack frames. And, surprise surprise, you do have it in the protocol and all adapters either could or do implement it.

@weinand I further suggest to make that totalFrames mandatory. I doubt any debug adapter wouldn't be able to fulfil that request. A stack trace's length is always at any debugger's fingertips.

@isidorn isidorn added this to the May 2016 milestone Apr 27, 2016
@isidorn
Copy link
Contributor

isidorn commented May 3, 2016

I am fine with experimenting with this. It sounds good to me.
@weinand let me know what you think, if making totalFrames mandatory is fine

@weinand
Copy link
Contributor

weinand commented May 3, 2016

There is a difference between explicit/voluntary lazy loading and implicit/involuntary lazy loading.
We've decided to use explicit/voluntary lazy loading because it gives the user explicit control when to load more frames. implicit/involuntary lazy loading would mean: if a user resizes the CALL STACK view to be large (or uses a big screen and VS Code in fullscreen mode), more than 20 frames will be loaded. If more frames are loaded automatically, it is more likely that node.js becomes unresponsive because one of those frames is too large. You can implement Joao suggestion but then we need an opt-out mechanism for adapters like node-debug.

We cannot change totalFrames (or any other attribute) to become mandatory without breaking all existing debugAdapters, so we need a 2.0 version of the debug protocol (which would mean you will have to support two versions). We can plan for this, but it is nothing that we will introduce lightly.

@isidorn
Copy link
Contributor

isidorn commented May 3, 2016

If nodeDebug would opt out of this then I propose to push this to the backlog.

@isidorn isidorn modified the milestones: Backlog, May 2016 May 3, 2016
@isidorn
Copy link
Contributor

isidorn commented May 26, 2016

Another issue with this idea is that it does not work nicely for multi-threaded applications

@isidorn isidorn removed their assignment May 26, 2016
@weinand
Copy link
Contributor

weinand commented May 26, 2016

@isidorn another good point.

@bpasero bpasero added the debug Debug viewlet, configurations, breakpoints, adapter issues label Nov 17, 2017
@isidorn isidorn added the *out-of-scope Posted issue is not in scope of VS Code label Nov 17, 2017
@vscodebot
Copy link

vscodebot bot commented Nov 17, 2017

This issue has been closed because the feature it requests is not within the scope of the product. See also our issue reporting guidelines.

Happy Coding!

@vscodebot vscodebot bot closed this as completed Nov 17, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality *out-of-scope Posted issue is not in scope of VS Code
Projects
None yet
Development

No branches or pull requests

4 participants