vscode-perl-debug
vscode-perl-debug copied to clipboard
raix
fix(variables): fix variable handling
Looking for feedback on the general approach here. This moves a lot of logic into a JSON-based protocol with the debugger through Perl code we inject into it (in the Devel::vscode namespace I made for this purpose). This fixes many issues with variable handling, including changes to the UI (different scopes / scope names) and probably a couple of things I noticed along the way. Probably still has some debug-code in it that ought to be removed before merging (edit: unclear if that is still true as of 2019-09-16). This also removes (as there is no need for it any longer) the variableParser.ts code and associated tests.
Fixes:
- https://github.com/raix/vscode-perl-debug/issues/79
- https://github.com/raix/vscode-perl-debug/issues/80
- https://github.com/raix/vscode-perl-debug/issues/81
- https://github.com/raix/vscode-perl-debug/issues/83
- https://github.com/raix/vscode-perl-debug/issues/85
Note that the Travis-CI failures for OS X are bogus, https://travis-ci.org/raix/vscode-perl-debug/jobs/514878456 for instance is supposed to test Perl 5.22, but ends up using Perl 5.18 like all tests do for that platform, some pass, some do not, and at this point I doubt that it is due to issues in the code or tests. When I add a couple of commits they will eventually all pass for no apparent reason. The same thing has happened for most of my PRs here.
@hoehrmann I've spent some time on fixing the test suite - now tests pass on both windows/linux/mac os travis didn't handle perl versions correctly - but now it's a true test matrix I've fixed some tests leaking a perl instance etc.
I think the work you have in this pr is valuable as it improves variable handling (maybe removes the dependency on padwalker?)
If you get time try merging in master then let's see if the test suite behaves
@raix: It does not remove the need for PadWalker, but does improve a number of things... Okay, will try to update the PR.
@raix: Give this a try. Test suite passes locally at least, and cursory check in vscode suggests it is still working, I also made one change that made me say it is not ready to be merged yet; I still suspect there is some debug-print-ish line somewhere that should be disabled, but haven't found it on short notice. Also, I still replace all of setVariableRequest which has undergone major changes, not sure which version is better. I probably won't be able to do much more in the forseeable future.
@hoehrmann both travis and appveyor are failing - I did fix the test suite to be more reliable so it's a matter of figuring out why the test is failing. (is this pr up to date with master?)
Note: When testing locally I created a script to move easily between versions - and I tested all versions locally on mac and windows - it's a hassle so I'm considering adding a vs code task to spin up the test suite in multiple terminals for that OS - would improve the development experience of working on this extension... I'll see if I can add it for windows and mac at least.