sbt bridge reporter #1530
sbt bridge reporter #1530
Conversation
MasseGuillaume
force-pushed
the
feature/sbt-bridge-reporter
branch
from
19265b9 to
7fea64c
Compare
MasseGuillaume
force-pushed
the
feature/sbt-bridge-reporter
branch
from
7fea64c to
251b385
Compare
| @@ -207,11 +207,28 @@ object DottyBuild extends Build { | |||
| ). | |||
| settings(publishing) | |||
|
|
|||
| // until sbt/sbt#2402 is fixed (https://github.com/sbt/sbt/issues/2402) | |||
| lazy val cleanBridge = TaskKey[Unit]("clean-sbt-bridge", "delete dotty-sbt-bridge cache") | |||
There was a problem hiding this comment.
I would call this cleanSbtBridge since that's how you would call this from sbt
There was a problem hiding this comment.
the convention is to use a-b-c
http://www.scala-sbt.org/0.13/sxr/sbt/Keys.scala.html#sbt.Keys.logLevel
There was a problem hiding this comment.
ah more or less: http://www.scala-sbt.org/0.13/sxr/sbt/Keys.scala.html#sbt.Keys.compileAnalysisFilename
:P. I will do as you said
There was a problem hiding this comment.
Sorry if I wasn't totally clear, I meant that the val should be called cleanSbtBridge, but I'm fine with changing the key name too :)
| IO.delete(file(home) / ".ivy2" / "cache" / "org.scala-sbt" / s"$org-$artifact-$dottyBridgeVersion-bin_${dottyVersion}__$classVersion") | ||
| IO.delete(file(home) / ".sbt" / "boot" / "scala-2.10.6" / "org.scala-sbt" / "sbt" / sbtV / s"$org-$artifact-$dottyBridgeVersion-bin_${dottyVersion}__$classVersion") | ||
| }, | ||
| publishLocal <<= publishLocal.dependsOn(cleanBridge), |
There was a problem hiding this comment.
<<= is deprecated: sbt/sbt#2716
There was a problem hiding this comment.
oki publishLocal := (publishLocal.dependsOn(cleanBridge)).value
| val ctx = (new ContextBase).initialCtx.fresh | ||
| .setSbtCallback(callback) | ||
| val freshContext = (new ContextBase).initialCtx.fresh | ||
| val ctx = freshContext.setSbtCallback(callback).setReporter(DelegatingReporter(delegate)) |
There was a problem hiding this comment.
The set* methods mutate the context and return this.type, so it's misleading to give two different names (freshContext and ctx) to the same context
There was a problem hiding this comment.
it crashes the compiler if I don't write it like this.
There was a problem hiding this comment.
OK, could you come up with a minimal example and open an issue? and leave a comment to indicate that you're working around a crash with the issue number
There was a problem hiding this comment.
strange, I just tested and it works fine.
| import xsbti.Maybe | ||
|
|
||
| object DelegatingReporter { | ||
| def apply(delegate: xsbti.Reporter) = new DelegatingReporter(delegate) |
There was a problem hiding this comment.
I would get rid of this and either make DelegatingReporter a case class or construct it with new
| else None | ||
|
|
||
| val file = | ||
| if(d.pos.source.file.exists) { |
There was a problem hiding this comment.
Style: if (, not if( (same issue in other parts of this file)
| val offset0 = pos.map(_.point) | ||
|
|
||
| val position = new xsbti.Position { | ||
| def line: Maybe[Integer] = maybe(pos.map(_.line)) |
There was a problem hiding this comment.
Maybe[Int] and Maybe[Integer] are the same thing so just use Maybe[Int]
There was a problem hiding this comment.
| case Some(s) => Maybe.just[T](s) | ||
| } | ||
| import java.lang.{ Integer => I } | ||
| private[this] def maybeInt(opt: Option[Int]): Maybe[I] = opt match { |
There was a problem hiding this comment.
This method is unnecessary, just use maybe above
There was a problem hiding this comment.
the compiler fails to infer with only maybe
There was a problem hiding this comment.
then write maybe[Int], please open an issue for this too if this is an issue specific to dotty
There was a problem hiding this comment.
scalac:
trait Position {
def offset(): Integer
}
object A {
val p = new Position {
def offset(): Int = 1
}
}overriding method offset in trait Position of type ()Integer;
method offset has incompatible type
| val position = new xsbti.Position { | ||
| def line: Maybe[Integer] = maybe(pos.map(_.line)) | ||
| def lineContent(): String = pos.map(_.lineContent).getOrElse("") | ||
| def offset(): xsbti.Maybe[Integer] = maybeInt(offset0) |
There was a problem hiding this comment.
xsbti.Maybe is imported so you can just write Maybe[...]
|
|
||
| val position = new xsbti.Position { | ||
| def line: Maybe[Integer] = maybe(pos.map(_.line)) | ||
| def lineContent(): String = pos.map(_.lineContent).getOrElse("") |
There was a problem hiding this comment.
Drop the empty param list () (same below)
| val file = | ||
| if(d.pos.source.file.exists) { | ||
| val r = d.pos.source.file.file | ||
| if(r == null) None |
There was a problem hiding this comment.
Just write Option(d.pos.source.file.file)
MasseGuillaume
force-pushed
the
feature/sbt-bridge-reporter
branch
from
ad9d8cf to
df0942f
Compare
|
@smarter all issues resolved! |
|
Though please squash the last commit with the review fixes in a previous one if possible |
MasseGuillaume
force-pushed
the
feature/sbt-bridge-reporter
branch
from
df0942f to
1e2bbb2
Compare
|
@smarter rebased |
|
/rebuild |
replaces #1528
dotty-bridge/scripted compilerReporter/simple