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

Allow a base address to be added to a file via a URI #170

Merged
merged 3 commits into from
Sep 8, 2020
Merged

Allow a base address to be added to a file via a URI #170

merged 3 commits into from
Sep 8, 2020

Conversation

haneefdm
Copy link
Contributor

I apologize if I didn't do things right. Even tabs vs. spaces. I am not a TypeScript expert either. Please review and I will do whatever is needed.

Basically, even if you are reading a file, the base address is not always zero. This is an awesome extension and I want to use it in a debugger. I am trying to address Issue #70 that I filed. It is only one part of the solution.

Bottom line: adds an offset to the addresses displayed

Yes, I have some imports for fs as I was using them for debugging and logging. As I found out, debugging two extensions at the same time was a bit challenging.

Btw, npm install fails for me because I don't have the nan package installed globally. I can fix package.json if you want.

@haneefdm haneefdm changed the title Allow a base address to be added to a binary file via a URI Allow a base address to be added to a file via a URI Aug 10, 2020
@haneefdm
Copy link
Contributor Author

I want to update the ChangeLog or release notes. Not sure what to update because of another PR #169 pending.

Please advise

@haneefdm
Copy link
Contributor Author

I am now wondering if I followed the process correctly. Could you guys please assign a reviewer(s) when you get a chance. The vscode-cpptools and MIEngine teams are also waiting. Thanks again

@heartacker
Copy link

@lramos15 please,3q

@lramos15
Copy link
Member

@haneefdm I promise this is on my radar. I no longer have merge access to this repo as I no longer work at Microsoft (I was a summer intern) and so I've reached out to the VS Code team to see if I'll be doing this review or if they will be doing it. I'm awaiting their response

@lramos15
Copy link
Member

lramos15 commented Sep 1, 2020

@mjbvz Do you have any suggestions on how to proceed with this PR?

@haneefdm
Copy link
Contributor Author

haneefdm commented Sep 3, 2020

I get a feeling this is a dead end. So sad. Such a nice extension and a needed one. Other teams within MS are also waiting for this and they have done a bunch of work as well.

Please, at least one response regarding the future of this extension would be nice. I totally understand corporate priorities change, etc., but please communicate

@mjbvz
Copy link
Contributor

mjbvz commented Sep 3, 2020

@lramos15 Does the change look good to you?

@lramos15
Copy link
Member

lramos15 commented Sep 3, 2020

@mjbvz Yes this looks good to me, the changes aren't very large. This would of course require a little bit of quick testing and a new version published but I have no issues with these changes

@mjbvz mjbvz merged commit 8b2e0b0 into microsoft:master Sep 8, 2020
@mjbvz
Copy link
Contributor

mjbvz commented Sep 8, 2020

Published in 1.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants