Support import command inside debug console#2596
Support import command inside debug console#2596DavyLandman merged 7 commits intousethesource:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2596 +/- ##
=======================================
Coverage 46% 46%
- Complexity 6650 6657 +7
=======================================
Files 793 793
Lines 65707 65719 +12
Branches 9842 9844 +2
=======================================
+ Hits 30612 30635 +23
+ Misses 32730 32725 -5
+ Partials 2365 2359 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jurgenvinju
left a comment
There was a problem hiding this comment.
Good feature! For a future refactoring we could use the command parser instead of the regex. That way we keep the Rascal syntax in one place (Rascal.rsc)
DavyLandman
left a comment
There was a problem hiding this comment.
If we continue using the regex (which as @jurgenvinju has some maintainability challenges) we should make sure it matches closer to what the rascal syntax allows for. Such as escapes of module names if they overlap with a keyword of the language.
…oderlein/rascal into support-import-debug-console
|
Fixed, the import now use RascalParser |
| IActionExecutor<ITree> actionExecutor = new NoActionExecutor(); | ||
| ITree tree = new RascalParser().parse(Parser.START_COMMAND, DEBUGGER_PROMPT_LOCATION.getURI(), command.toCharArray(), INodeFlattener.UNLIMITED_AMB_DEPTH, actionExecutor, new DefaultNodeFlattener<IConstructor, ITree, ISourceLocation>(), new UPTRNodeFactory(false)); | ||
| Command stat = new ASTBuilder().buildCommand(tree); | ||
| if (!stat.isImport()) { | ||
| return null; | ||
| } | ||
| Environment oldEnvironment = evaluator.getCurrentEnvt(); | ||
| evaluator.setCurrentEnvt(evaluator.__getRootScope()); // For import we set the current module to the root to reload modules properly | ||
| Result<IValue> result = evaluator.eval(evaluator.getMonitor(), command, DEBUGGER_PROMPT_LOCATION); | ||
| evaluator.setCurrentEnvt(oldEnvironment); | ||
| return result; |
There was a problem hiding this comment.
We should make sure that ParseError exceptions are caught, so that if something is not a command, it doesn't bubble up as an exception?
There was a problem hiding this comment.
Since this function is called inside the evaluate function that catch and display nicely ParseErrror, this is already the case.
There was a problem hiding this comment.
My thinking was: now the following if body (that does evaluator.eval(...)) will never run. Which will most likely not happen, as it also parses the string in the same way. lets add a comment or something? it smells a bit fishy that we just assume line 327 is the same parse as the one inside Evaluator::eval
There was a problem hiding this comment.
Ok I understand, I added a comment to clarify assumption on the call.
This PR fix an error on import command inside the debug console of DAP.
The command import module at REPL level, and reload the module if it is already imported.