X Tutup
The Wayback Machine - https://web.archive.org/web/20220903062917/https://github.com/microsoft/vscode/pull/129797
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

Add a special entry when disassembly is unavailable #129797

Merged
merged 3 commits into from Aug 23, 2021

Conversation

xisui-MSFT
Copy link
Contributor

@xisui-MSFT xisui-MSFT commented Jul 29, 2021

This PR fixes #129526

Add a special entry to show text 'Disassembly not available' when the disassembly view is empty.

@xisui-MSFT
Copy link
Contributor Author

xisui-MSFT commented Jul 29, 2021

@isidorn @weinand Can you please take a look? Thanks!

Draft PR because I would like to add a accessibility change for this after merging #129788

@isidorn
Copy link
Contributor

isidorn commented Jul 30, 2021

I am not sure this is the best approach. Visually it might not look so good, and users might need to close it manually which can be a pain.
Let's assign to August so we can discuss this more.

First important thing to figure out, do users want to inspect Disassembly after they stopped debugging. If YES, I propose the following:

  1. After debugging is stopped, disassembly view is still there
  2. When debugging is started again, the disassembly view is reused and filled with the new content
  3. On vscode close we close the disassembly view. It should not survive across vscode sessions

If NO

  1. After debugging is stopped we automatically close disassembly view

In both these cases we would not need a placeholder solution like the one proposed in this PR.
Since we have a disassembly issues with 300+ upvotes, how about we ask the users some questions there?

fyi @yuehuang010 @gregg-miskelly

@isidorn isidorn assigned isidorn and unassigned eamodio Jul 30, 2021
@isidorn isidorn added debug Debug viewlet, configurations, breakpoints, adapter issues under-discussion Issue is under discussion for relevance, priority, approach labels Jul 30, 2021
@isidorn isidorn added this to the August 2021 milestone Jul 30, 2021
@xisui-MSFT xisui-MSFT changed the title Dev/xisui/fix 129526 Add a special entry when disassembly is unavailable Aug 11, 2021
@xisui-MSFT
Copy link
Contributor Author

xisui-MSFT commented Aug 11, 2021

@isidorn Yes users may want to inspect disassembly after they stopped debugging.

When debugging is started again, the disassembly view is reused and filled with the new content

We can't do this because disassembly is only available on debug pause, such as a breakpoint hit.

@isidorn
Copy link
Contributor

isidorn commented Aug 12, 2021

@xisui-MSFT okey, but once debugging is paused again please re-use the current disassembly view.
So there is a transition period: while a new session is starting and before it is paused. In that case I suggest to simply update the title of the view to be Dissasembly View(outdated) or Dissasembly View(stale) or something like that.

And also make sure to close it on vscode shutdown so it is not left over for user to manage.

@xisui-MSFT
Copy link
Contributor Author

xisui-MSFT commented Aug 12, 2021

@isidorn Sometimes the view could be empty when the debugger or debug adapter have some bugs. For example, microsoft/vscode-cpptools#7960.

We were planning to add more special entries as we move forward, and so have a consistent way of showing errors or hints for users. As for this message in this PR, we could keep it as it is or probably change it to something like "Pause debugging or hit a breakpoint for disassembly".

once debugging is paused again please re-use the current disassembly view.

Yes, there is only one disassembly view currently.

And also make sure to close it on vscode shutdown so it is not left over for user to manage.

I'm not sure if I need to do anything extra here? The default behavior seems to be closing the view when closing VS Code.

@isidorn
Copy link
Contributor

isidorn commented Aug 13, 2021

@xisui-MSFT ok, all makes sense.

Let me know once this PR is ready to be reviewed / merged. Thanks

@xisui-MSFT xisui-MSFT marked this pull request as ready for review Aug 20, 2021
@xisui-MSFT
Copy link
Contributor Author

xisui-MSFT commented Aug 20, 2021

While ready for review, it's hard for me to do complex changes right now because of this issue: #130947

@isidorn
Copy link
Contributor

isidorn commented Aug 23, 2021

@xisui-MSFT thanks for this PR, this looks good to me. Let's merge it in.
As for #130947 I see that Deepak has responded. Hopefully that helps.

@isidorn isidorn merged commit 5489d1b into microsoft:main Aug 23, 2021
7 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 2021
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 under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
X Tutup