Add console to bridge #1314
Add console to bridge #1314
Conversation
| classpathString: String, | ||
| initialCommands: String, | ||
| cleanupCommands: String, | ||
| loader: ClassLoader, |
There was a problem hiding this comment.
loader isn't passed to the REPL, is that a TODO? If yes that should be noted in a comment
There was a problem hiding this comment.
Good point, it isn't necessary to pass the loader (here atm) - but if a modified loader is passed somehow, it makes sense that it be propagated to the REPL. Will add that in the morning :) thanks!
felixmulder
force-pushed
the
topic/bridge-repl
branch
from
09f2195 to
b63b840
Compare
|
|
||
| import scala.util.Try | ||
|
|
||
| class CombinedClassLoader(loaders: List[ClassLoader]) extends ClassLoader { |
There was a problem hiding this comment.
Yes, this guy needs to go away - not sure how yet though. Every other solution seems to create a circular dependency on sbt 👎
There was a problem hiding this comment.
Why is this needed? Does scalac do something similar?
There was a problem hiding this comment.
They just set it as the class loader - but that doesn't work for us. So I decided that it would be a good idea to merge the ClassLoaders and look through both of them when searching for classes (which works).
But I don't like this solution - if you have a better one - I'm all ears.
There was a problem hiding this comment.
Why doesn't it work for us? Can't we replace parentClassLoader in val classLoader with the classloader given by sbt?
There was a problem hiding this comment.
In general, I would advise against making a custom subclass of ClassLoader because it can very easily not do what you think it should do, and it's hard to debug, but I don't know enough about the situation here to say for sure.
felixmulder
force-pushed
the
topic/bridge-repl
branch
2 times, most recently
from
28706f6 to
9e76529
Compare
felixmulder
force-pushed
the
topic/bridge-repl
branch
from
9e76529 to
22b40bf
Compare
Since you still need to depend on multiple artifacts from within the dotty project, a unified project is not feasible. A plugin would work better of course.
felixmulder
changed the title
|
This feels stable enough for review now - @DarkDimius |
| val parent = new URLClassLoader(compilerClasspath.toArray, parentClassLoader) | ||
| new AbstractFileClassLoader(virtualDirectory, parent) | ||
| lazy val parent = new URLClassLoader(compilerClasspath.toArray, | ||
| classOf[Interpreter].getClassLoader) |
There was a problem hiding this comment.
why Interpreter instead of CompilingInterpreter?
There was a problem hiding this comment.
Good question - I think this was originally written by @odersky
|
LGTM, otherwise. |
This PR does two things:
consolefrom within an sbt-session that is using the dotty-bridgesbt dotty-core/publishLocalor to deploy real snapshots:sbt dotty-core/publish