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

Fixes #415

Merged
merged 4 commits into from
Mar 11, 2024
Merged

Fixes #415

merged 4 commits into from
Mar 11, 2024

Conversation

oltolm
Copy link
Contributor

@oltolm oltolm commented Mar 7, 2024

Hello,

I tried ND on Windows with VSCode 1.87.1 and GDB 14.2 and ran into a lot of problems. I fixed some of them:

  1. the debugger can now display thread names
  2. local variables actually work, they were completely broken
  3. I changed the default to "prettyPrinters"
  4. prevented the error message "Selected thread is running." from displaying, it's not an error
  5. cleaned up the code a bit in a separate commit

It still does not display global or member variables, maybe I will fix it later.

@codecov-commenter
Copy link

codecov-commenter commented Mar 7, 2024

Codecov Report

Attention: Patch coverage is 0% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 20.49%. Comparing base (a6f48d6) to head (aee5f71).

Files Patch % Lines
src/backend/mi2/mi2.ts 0.00% 9 Missing ⚠️
src/backend/backend.ts 0.00% 7 Missing ⚠️
src/mibase.ts 0.00% 4 Missing ⚠️
src/backend/mi2/mi2lldb.ts 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #415      +/-   ##
==========================================
- Coverage   20.55%   20.49%   -0.06%     
==========================================
  Files          14       14              
  Lines        1800     1805       +5     
  Branches      388      390       +2     
==========================================
  Hits          370      370              
- Misses       1385     1390       +5     
  Partials       45       45              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/mibase.ts Outdated
}).catch(error => {
}).catch((error: MIError) => {
if (error.message === 'Selected thread is running.') {
console.error(error);
Copy link
Owner

Choose a reason for hiding this comment

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

console stuff is not logged on the client, it's just lost

you probably want to either use the DAP error response or send it to the output using this.handleMsg("stderr", "...")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can see the console when debugging the extension. I wanted to ignore this message because it's not an error.

Copy link
Owner

@WebFreak001 WebFreak001 Mar 7, 2024

Choose a reason for hiding this comment

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

You will have to either do sendResponse or sendErrorResponse here though, otherwise it's a hanging response that can cause issues with various DAP consumers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I used this.handleMsg like you suggested.

src/backend/mi2/mi2.ts Outdated Show resolved Hide resolved
src/backend/mi2/mi2.ts Outdated Show resolved Hide resolved
src/backend/mi2/mi2.ts Outdated Show resolved Hide resolved
@oltolm
Copy link
Contributor Author

oltolm commented Mar 11, 2024

There were linting problems, so I ran npm update --save and then I had to upgrade typescript npm install typescript@4.9.3 --save-dev.

Copy link
Owner

@WebFreak001 WebFreak001 left a comment

Choose a reason for hiding this comment

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

can you revert the package lock changes because of npm update --save and only run do the typescript update? The other stuff should not change

@oltolm
Copy link
Contributor Author

oltolm commented Mar 11, 2024

can you revert the package lock changes because of npm update --save and only run do the typescript update? The other stuff should not change

I reverted the changes and then I just did npm install typescript@4.9.3 --save-dev.

@WebFreak001 WebFreak001 merged commit 04068e9 into WebFreak001:master Mar 11, 2024
4 checks passed
@oltolm oltolm deleted the fixes branch March 11, 2024 22:42
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.

3 participants