feat: add breakpoint column support#224
Conversation
|
This is a large change that should be covered by tests. Please add tests to VSCodeTestSrcBreakpointResolve. You will probably need to add a Please cover all possible cases (proper resolve with column inside statement, several breakpoints set to the same statement, etc.), for example:
First set with column 0, then with 17, then with 23.
First set with column 17, then with 0, then with 23. And so on... |
|
Yep I agreee, ill make some tests. I did try to run the tests because I considered adding some but ran into some issues with dotnet runtime 3.1 and globalization issues. Ill figure that out and cover it with tests |
|
You could replace "netcoreapp3.1" with "net10.0" in all ./test-suite files and use .NET 10 as I do ;) |
|
Ah lol didnt think of that, is it worth considering upstreaming that change or does it break CI stuff? |
|
CI tests with .NET 3.1, 6.0, 8.0 and 10.0 now in the same way as I wrote - it replace "netcoreapp3.1" with the framework it needs for the current test loop. :) |
|
Sorry it took a while, I added some tests now. let me know what you think |
|
@viewizard nudge |
|
Looks good. I'm fixing the Samsung CI now. As soon as it's up, we can start testing these changes. |
My attempt at implementing what was discussed in #219 and resolves #83.
As discussed with @viewizard, the approach is: use the column to pick the right IL offset while keeping the 1 breakpoint per line rule. Column 0 falls back to existing behavior.
I had to make some heuristics changes in SymbolReader.cs, feel free to sanity check me.
When a column is provided, sequence point selection for the outer method prefers the SP whose StartColumn is closest to (but not exceeding) the target column. A cursorInsideNested guard ensures that when the cursor is positioned inside a lambda's column range, the breakpoint lands in the lambda rather than the outer call site.
column and endColumn are now returned in the DAP setBreakpoints response.
The test case from #83 now works. Tested locally in neovim with various changes and seems to work as expected
Keep in mind I usually never code in C++. If there are edge cases I've missed feel free to tag me and I'll test and fix.
Fixes #83