-
Notifications
You must be signed in to change notification settings - Fork 82
Support import command inside debug console #2596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Support import command inside debug console #2596
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2596 +/- ##
=======================================
- Coverage 46% 46% -1%
+ Complexity 6637 6635 -2
=======================================
Files 793 793
Lines 65695 65707 +12
Branches 9840 9842 +2
=======================================
Hits 30500 30500
- Misses 32835 32849 +14
+ Partials 2360 2358 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jurgenvinju
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.