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

In debug mode, arguments are by default collapsed #32199

Closed
straxhaber opened this issue Aug 9, 2017 · 11 comments
Closed

In debug mode, arguments are by default collapsed #32199

straxhaber opened this issue Aug 9, 2017 · 11 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues info-needed Issue requires more information from poster

Comments

@straxhaber
Copy link

  • VSCode Version: 1.16.0
  • OS Version: OSX 10.12.6

Steps to Reproduce:

  1. Run code in debug mode
  2. Look at "Variables" section

Reproduces without extensions: Yes

@vscodebot vscodebot bot added the debug Debug viewlet, configurations, breakpoints, adapter issues label Aug 9, 2017
@straxhaber
Copy link
Author

This issue affects usability. While debugging, arguments to a function are critical for understanding what is going on. They are variables that affect execution. It is annoying to have to expand the section every time the call stack is changed. At the very least, this should be a configurable option. However, I propose that the default should be that the variables' sections are expanded for a better development experience.

I am completely new to JavaScript/TypeScript, but I have created a patch that seems to fix the issue. I would appreciate a code review! I will submit it shortly.

@straxhaber
Copy link
Author

straxhaber commented Aug 9, 2017

Cross-posting from pull request:

When debugging, argument variables are just as important as local variables. Both sections should be viewable while debugging. Instead, the current version of the code only expands the first dropdown and requires the user to manually expand the argument variables section. This is a tedious requirement and a poor user experience. Instead, the default should be to expand both the local variables and argument variables sections. I have already experienced a scenario where a name conflict between argument and local variables resulted in a bug in my code. Not seeing the argument variables automatically made it a bit harder to debug. Furthermore, when debugging the code I keep having to expand the argument variables section to see what the value of the variables are.

Please note that this is my first time writing any JavaScript or TypeScript code. Of course, I did my best to write clean code, and I both ran the unit tests and tested the functionality in a fresh build of VS Code. However, I would appreciate a code review and am happy to make any modifications necessary.

Best,
Matt

@weinand weinand added the info-needed Issue requires more information from poster label Aug 9, 2017
@weinand
Copy link
Contributor

weinand commented Aug 9, 2017

I cannot reproduce the problem you are seeing (or I do not really understand it).
Please provide more detailed steps that explain how to reproduce this.

@straxhaber
Copy link
Author

Screenshots:
screenshot 2017-08-09 16 00 41
screenshot 2017-08-09 16 00 59

Expected behavior:

  • "Arguments" sub-section in Variables defaults to expanded

Actual behavior:

  • "Arguments" sub-section in Variables defaults to collapsed

This is a U/X bug. Argument variables are just as important (if not more important) to debugging, and a user of Visual Studio Code shouldn't have to manually expand the "Arguments" section each time. It should default to being expanded.

@straxhaber
Copy link
Author

I hope that clears things up. Let me know if you need further clarification. The pull request I submitted changes this behavior and (in my opinion) improves the U/X.

@weinand
Copy link
Contributor

weinand commented Aug 9, 2017

Before spending time on providing a PR, we recommend to first file an issue that explains the problem with reproducible steps and states the intent that you want to submit a PR.

I could not reproduce the problem because I didn't know that you are seeing this problem with python and not with one of the built-in languages of VS Code.

The "Arguments" scope is a Python contribution (VS Code does not know the semantics of the scope sections, so there is no built-in 'Arguments' scope). Trying to expand it with a "fix" in VS Code will affect all debugger extensions. Such a fundamental change should not be done lightly.

I suggest that you file this issue against the Python extension first. You can use this Issue mover tool: https://github-issue-mover.appspot.com.

The outcome of this could very well be that we need a more flexible way to auto-expand specific scopes in the Variables view. But this will require additions to the debug protocol (which I can take care of).

@straxhaber
Copy link
Author

@weinand I'm responding to this and your comment in #32204 here.

First of all, no worries on the time spent providing a PR. My main motivation was to learn a bit of JavaScript/TypeScript while exploring the code-base. I accomplished that even if my PR isn't used!

As to your point in issue #32204 about performance, I thought the .expensive attribute was there to determine if the scope is computationally expensive to load. Is that not the case? If it is the case, my modification still avoids expanding any scopes that are marked as expensive (the same way the previous version of the code did).

it's worth noting that my code doesn't do anything new to scopes. The previous code also automatically expanded a scope; the only difference is that the previous code arbitrarily discriminates in favor of opening just the first scope. The first scope is no more or less likely to be expensive to load than any other scope. Is there a reason that it is more appropriate to expand the first scope than other scopes?

Also, while the Arguments scope is a Python contribution, this doesn't seem to be a Python-specific issue. I would argue that in general all "Variables" sections that are relevant to a debug context (unless "expensive") should be displayed, and VS Code should not assume that the first scope is any more relevant than other scopes. Is there a reason to think otherwise? Either way, I would welcome your thoughts on the best way to implement this in VS Code or the Python Extension.

@straxhaber
Copy link
Author

Also, I would argue that if anything the "Local Variables" view (the default first section that is automatically expanded by the previous version of the code) is just as much if not more likely to be expensive to load. If automatic expansion of scopes is a performance risk, then this behavior should be disabled as well, but I would argue that is the wrong direction to take this.

@straxhaber
Copy link
Author

One more addition to this: VS Code automatically collapses all scopes besides the first every time the debugger is advanced. This overrides even a user's decision about whether to expand other scopes. If I'm interpreting the code correctly (quite possible I'm not since I'm not familiar with JS/TS), then this is being done by VS Code not the Python plugin.

@weinand
Copy link
Contributor

weinand commented Aug 10, 2017

If a scope collapses while stepping, this is a problem of the backend (debug adapter inside the debug extension). VS Code tries to preserve the expansion state of the tree while stepping.

Please always compare the behaviour of at least two extensions before generalising some observed behaviour as VS Code's fault. In your case I suggest to compare the behaviour of the Python debugger with the Node debugger.

@vscodebot vscodebot bot closed this as completed Aug 17, 2017
@vscodebot
Copy link

vscodebot bot commented Aug 17, 2017

This issue has been closed automatically because it needs more information and has not had recent activity. Please refer to our guidelines for filing issues. Thank you for your contributions.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
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 info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

2 participants