Skip to content

Suppressing unneeded error message when hovering#366

Merged
jreineckearm merged 16 commits intoeclipse-cdt-cloud:mainfrom
omarArm:main
May 21, 2025
Merged

Suppressing unneeded error message when hovering#366
jreineckearm merged 16 commits intoeclipse-cdt-cloud:mainfrom
omarArm:main

Conversation

@omarArm
Copy link
Contributor

@omarArm omarArm commented May 12, 2025

Previous behaviour:

when hovering over non variables, i.e. comments or keywords (for, while, etc.), an error message appeared "-var-create: unable to create variable object". Although this is technically a correct behaviour, it was frustrating to users to see an error message just because they are hovering over comment sections in their code.

Current behaviour:

This error message is now suppressed when hovering over comments.

@jreineckearm
Copy link
Contributor

Addresses #363

Copy link
Contributor

@jreineckearm jreineckearm left a comment

Choose a reason for hiding this comment

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

Changes LGTM!
@jonahgraham , @asimgunes , do you have any concerns with this? Planning to merge otherwise tomorrow night (CET) or on Wednesday.

@jreineckearm
Copy link
Contributor

@omarArm , automated tests failed. Please have a look. Be aware though that running them on macOS isn't straight forward. Still working with @thorstendb-ARM to get them going.

Copy link
Contributor

@jreineckearm jreineckearm left a comment

Choose a reason for hiding this comment

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

Revoking approval due to failing tests.

@omarArm
Copy link
Contributor Author

omarArm commented May 13, 2025

Hi, the tests are failing because my changes are changing the expected behaviour when a failing evaluate request is being triggered. That is by design on my side, I think the test script itself should change

Copy link
Contributor

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

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

It looks like multiple topics are included in this PR. I added a comment on the part that I can identify as applicable to the PR description and related issue. All the other stuff about breakpoints seem to be a different topic.

@omarArm omarArm requested a review from jreineckearm May 19, 2025 12:54
Copy link
Contributor

@jreineckearm jreineckearm left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. LGTM!
Let's make sure to squash and merge this PR and clean up the commit message for that.

Will leave this PR unmerged until tomorrow (CET) to give others a chance to review because the changes are user visible (CC: @jonahgraham , @asimgunes ).

@jreineckearm jreineckearm merged commit a2387ed into eclipse-cdt-cloud:main May 21, 2025
4 checks passed
jonahgraham added a commit to jonahgraham/cdt-gdb-vscode that referenced this pull request May 21, 2025
  - [Add device reset](eclipse-cdt-cloud/cdt-gdb-adapter#365)
  - [Suppressing unneeded error message when hovering](eclipse-cdt-cloud/cdt-gdb-adapter#366)
jreineckearm added a commit to eclipse-cdt-cloud/cdt-gdb-vscode that referenced this pull request May 22, 2025
- [Add device reset](eclipse-cdt-cloud/cdt-gdb-adapter#365)
  - [Suppressing unneeded error message when hovering](eclipse-cdt-cloud/cdt-gdb-adapter#366)

Co-authored-by: Jens Reinecke <jens.reinecke@arm.com>
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