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.

Sign up for GitHub

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

Jump to bottom

avoid "Unable to open file" popups caused by missing source code #1038

Merged
merged 6 commits into from
Jan 3, 2022
Merged

avoid "Unable to open file" popups caused by missing source code #1038

merged 6 commits into from
Jan 3, 2022

Conversation

Trass3r
Copy link
Contributor

@Trass3r Trass3r commented Aug 27, 2020 •
edited
Loading

Preliminary solution until full disassembly debugging is implemented.
@WardenGnaw

@WardenGnaw WardenGnaw self-requested a review August 27, 2020 20:33
@WardenGnaw WardenGnaw self-assigned this Aug 27, 2020
@WardenGnaw WardenGnaw changed the base branch from master to main September 11, 2020 21:28
@Trass3r Trass3r mentioned this pull request Sep 21, 2020
Closed
@Trass3r
Copy link
Contributor Author

Trass3r commented Dec 22, 2020 •
edited
Loading

@gregg-miskelly maybe something like this?
At least it's easily readable.

gregg-miskelly
gregg-miskelly approved these changes Jan 8, 2021
View reviewed changes
Copy link
Member

@gregg-miskelly gregg-miskelly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me aside from those small changes.

src/OpenDebugAD7/TextPositionTuple.cs Outdated Show resolved Hide resolved
src/OpenDebugAD7/AD7DebugSession.cs Outdated Show resolved Hide resolved
@gregg-miskelly
Copy link
Member

@WardenGnaw do you want to review this?

@WardenGnaw
Copy link
Member

WardenGnaw commented Jan 21, 2021 •
edited
Loading

This commit
converts it to be using async/await with Tasks instead of Parallel.ForEach

Still need to test with VS Code.

@WardenGnaw
Copy link
Member

@Trass3r I'm getting a remote rejected on pushing to the deemphasize branch.

@Trass3r
Copy link
Contributor Author

Trass3r commented Jan 21, 2021

Fixed.

@WardenGnaw
Copy link
Member

Old Behavior when stepping into a function you do not have source:
Popup:
image

New Behavior:
Nothing but callstack shows there is no source for the top frame.
image

@ntd252
Copy link

ntd252 commented Feb 1, 2021 •
edited
Loading

Hello, I hope my comment is in the right place.
I'm using MinGW-w64 (from MSYS2), debug and build settings are showed in video. I get the error:
Unable to open 'crtexe.c': Unable to read file 'c:\_\M\mingw-w64-crt-git\src\mingw-w64\mingw-w64-crt\crt\crtexe.c' (Error: Unable to resolve non-existing file 'c:\_\M\mingw-w64-crt-git\src\mingw-w64\mingw-w64-crt\crt\crtexe.c').
I hope the final fix will come. Thank you!
https://user-images.githubusercontent.com/36019046/106465437-89076000-64cc-11eb-93d3-3ad49b80bf99.mp4

c++bug

@Trass3r
Copy link
Contributor Author

Trass3r commented May 4, 2021

@WardenGnaw wasn't this finished already (apart from Gregg's comment)?

@daniel-brosche
Copy link

is this still in progress?

@Trass3r
Copy link
Contributor Author

Trass3r commented Jul 14, 2021

It's been ready for a while, rebased it and addressed the last items..

@daniel-brosche
Copy link

@WardenGnaw please review this feature, there are so many poeple waiting for this (since years) -> microsoft/vscode-cpptools#811 (comment)

@evdenaxas

This comment has been minimized.

Sign in to view
@Trass3r
Copy link
Contributor Author

Trass3r commented Jul 15, 2021 •
edited
Loading

Something seems to have changed though in vscode with main MIEngine. For me when stepping over a throw I don't get the popup anymore.
It just opens the file showing Could not load source './signal/../sysdeps/unix/sysv/linux/raise.c': 'SourceRequest' not supported..

@evdenaxas
Copy link

Which version @Trass3r ? Mine is 1.58.1
Version: 1.58.1
Commit: 2aeda6b18e13c4f4f9edf6667158a6b8d408874b
Date: 2021-07-13T06:20:02.397Z
Electron: 12.0.13
Chrome: 89.0.4389.128
Node.js: 14.16.0
V8: 8.9.255.25-electron.0
OS: Linux x64 5.8.0-59-generic

@Trass3r
Copy link
Contributor Author

Trass3r commented Jul 15, 2021

Same version. On WSL2.

@daniel-brosche
Copy link

@Trass3r + @WardenGnaw is there still something missing, what is needed to get this done?

@daniel-brosche
Copy link

@WardenGnaw please merge it

@diditforlulz273
Copy link

would you merge it at last? I'm aging waiting for it, and I'm not a wine to get better with years.

@daniel-brosche
Copy link

I*m really looking really forward to use this feature...

@Greg-MV
Copy link

Greg-MV commented Oct 19, 2021

Guys we'd really like this feature.

@evdenaxas

This comment has been minimized.

Sign in to view
@WardenGnaw WardenGnaw merged commit 2586fe8 into microsoft:main Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chuckries chuckries chuckries left review comments

@gregg-miskelly gregg-miskelly gregg-miskelly approved these changes

@WardenGnaw WardenGnaw Awaiting requested review from WardenGnaw

Assignees

@WardenGnaw WardenGnaw

Labels
Feature Request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@Trass3r @gregg-miskelly @WardenGnaw @ntd252 @daniel-brosche @evdenaxas @diditforlulz273 @Greg-MV @chuckries