eclipse-cdt-cloud / cdt-gdb-adapter

CDT GDB Debug Adapter
Eclipse Public License 2.0
27 stars 40 forks source link

Support SteppingGranularity & instruction stepping #309

Closed colin-grant-work closed 8 months ago

colin-grant-work commented 8 months ago

This PR adds support for SteppingGranularity to the next and stepIn requests, e.g. from the VSCode disassembly view.

To do:

jonahgraham commented 8 months ago

Battle of the formatters :-/

Who is battling? "yarn format" uses prettier, I have prettier installed in the IDE so it formats on save. Maybe we should add it to recommendations in extension.json?

colin-grant-work commented 8 months ago

Who is battling? "yarn format" uses prettier, I have prettier installed in the IDE so it formats on save. Maybe we should add it to recommendations in extension.json?

Yeah, it's the battle between VSCode's default formatter and Prettier. It would likely be helpful to add Prettier to the recommendations & add

"editor.defaultFormatter": "esbenp.prettier-vscode"

to the settings.json. Then the only thing missing for a perfect set-up is for contributors to set other things as the default formatter elsewhere so having the Prettier plugin installed doesn't make formatting harder in other repos. Or to set Prettier as disabled in other workspaces and enabled here. I had things more nicely set up for a while, but I jump around a lot and use a lot of ad-hoc multi-root workspaces, so it got messy again.

The difficulty with Prettier is that it's so versatile, so setting it up is easy with the preference above, but disabling it means setting a bunch of language-specific things:

[typescript]: {editor.defaultFormatter: notPrettier1}
[javascript]: {editor.defaultFormatter: notPrettier1}
[markdown]: {editor.defaultFormmatter: notPrettier2}
[json]: {editor.defaultFormatter: notPrettier3}
colin-grant-work commented 8 months ago

Alright, @jonahgraham, I seem to have finally managed to write tests that weren't foolishly specific to my machine, if you wouldn't mind taking a look.

jonahgraham commented 8 months ago

Your change looks good to me, I have made a couple of commits to make the test code more generic. Please see commits for details as to why I suggest such changes.

colin-grant-work commented 8 months ago

Thanks for the fixes! Could you explain the significance of this part?

make the line of code the same as opening brace to account for different gdb/gcc combinations

I saw that comment / commit message in other test cases, but I didn't and still don't understand why that particular change is important.

jonahgraham commented 8 months ago

For a function like this:

int method(void) {
  call_something();
}

some combinations of GCC/GDB versions consider line 1 (line with {) the first line of the method and some consider it line 2 (line with first instructions). So to avoid cases where we want to detect we are on the first source line of a method we place { and first code on same source line so we don't have to identify the differences in the tests.

If you want I can dig through my history for which combinations do which thing.

Is that change ok to you?

colin-grant-work commented 8 months ago

Yeah, definitely OK - just wanted to understand better why it was needed. Thanks!

colin-grant-work commented 8 months ago

Sorry for the multiple pushes - was confused about where your commits lived, but I believe I've got them now.

jonahgraham commented 8 months ago

Sorry for the multiple pushes - was confused about where your commits lived, but I believe I've got them now.

No worries - it is sort of weird that reviewers can push to contributor branches :-)