-
Notifications
You must be signed in to change notification settings - Fork 7
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
New rascal-core & rascal #394
base: main
Are you sure you want to change the base?
Conversation
@@ -224,7 +226,7 @@ public void warning(WarningMessage param) { | |||
|
|||
@Override | |||
public void registerLocations(RegisterLocationsParameters param) { | |||
new BasicIDEServices(null).registerLocations(param.getScheme(), param.getAuthority(), param.getMapping()); | |||
new BasicIDEServices(new PrintWriter(PrintWriter.nullWriter()), new NullRascalMonitor()).registerLocations(param.getScheme(), param.getAuthority(), param.getMapping()); |
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.
Perhaps we can just inline the default method here, because it skips all the monitor variables anyway:
default void registerLocations(IString scheme, IString auth, IMap map) {
URIResolverRegistry.getInstance().registerLogical(new LogicalMapResolver(scheme.getValue(), auth.getValue(), map));
}
default void unregisterLocations(IString scheme, IString auth) {
URIResolverRegistry.getInstance().unregisterLogical(scheme.getValue(), auth.getValue());
}
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.
that would cause a clone of that code, I would rather not.
boolean jobSuccess = false; | ||
try { | ||
monitor.jobStart("Loading " + label); | ||
services.jobStart("Loading " + label); |
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.
This "Loading " job does not have steps. Better remove it; it will make more sense if we leave it to the internal evaluators "loading modules" job.
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.
There is a doImport
call in this method that can now take the entire array of imports
as a parameter. So the loop can be dropped and the internal monitor can do each module as a jobStep
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.
I'm going to add job steps, as the reason why this monitor is the top-level one is that it's part of the title of this progress bar. So I want a pretty one, and not one that says which module it's loading (you can still see that in the details.)
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.
Reason's I'm not doing that doImport
call:
- the current code allows me to print a more accurate error message of which root module went wrong
- the
doImport
with the varargs is not in a release rascal jar yet, so we can always migrate to that later.
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.
Just know that every doImport call will now produce its own progress bar; or we go back to the steps without starts situation.
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.
Note that the exception will tell you which module went wrong anyhow.
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 can drop the empty "Loading ... " job (see comment)
- use
doImport(string array or ...param)
instead of looping ourselves for better monitors.
|
This brings:
to Rascal.
It's still in Draft as stuff doesn't work yet, I just got it to compile.