[WIP] #1579 Adapt the sbt bridge for the new error message

[slothspot]

I started adaptation of sbt bridge.
Right now I have following output for next samples:
case class Foo
results in (with explain):

[error] /Users/dmitry/Projects/dotty-example-project/src/main/scala/Main.scala:10:
[error] case class Foo
[error]            ^^^
[error] A case class must have at least one parameter list
[error] Foo must have at least one parameter list, if you would rather
[error] have a singleton representation of Foo, use a "case object".
[error] Or, add an explicit `()' as a parameter list to Foo.

results in (without explain):

[error] /Users/dmitry/Projects/dotty-example-project/src/main/scala/Main.scala:10:
[error] case class Foo
[error]            ^^^
[error] A case class must have at least one parameter list

println("msg", 123, None)
results in (with explain):

[error] /Users/dmitry/Projects/dotty-example-project/src/main/scala/Main.scala:23:
[error]     println("msg", 123, None)
[error]     ^^^^^^^
[error] none of the overloaded alternatives of method println in object Predef with types
[error]  (x: Any)Unit
[error]  ()Unit
[error] match arguments (String("msg"), Int(123), None.type)

results in (without explain):

[error] /Users/dmitry/Projects/dotty-example-project/src/main/scala/Main.scala:23:
[error]     println("msg", 123, None)
[error]     ^^^^^^^
[error] none of the overloaded alternatives of method println in object Predef with types
[error]  (x: Any)Unit
[error]  ()Unit
[error] match arguments (String("msg"), Int(123), None.type)

case class Bar(s: String,
  i: Itn,
  b: Boolean)

results in (with explain):

[error] /Users/dmitry/Projects/dotty-example-project/src/main/scala/Main.scala:13:
[error]   i: Itn,
[error]      ^^^
[error] not found: type Itn
[error] An identifier for `type Itn` is missing. This means that something
[error] has either been misspelt or you're forgetting an import

results in (without explain):

[error] /Users/dmitry/Projects/dotty-example-project/src/main/scala/Main.scala:13:
[error]   i: Itn,
[error]      ^^^
[error] not found: type Itn

At this moment I would like to receive suggestions about errors output format:
should it be the same as used by dotty or proposed one is acceptable? (I opt for second because of sbt messages decoration)

[felixmulder]

I think it should present the same output as dotc. We worked hard to make sure that the output from dotc is readable and well separated such that it is easy to know where errors end and begin. There's no point in better error messages if you can't tell where one starts and the other begins 😃

Instead of implementing a "new" reporter, I would look at adapting the ConsoleReporter such that you can get the complete String and pass it to the delegating reporter. Perhaps this could simply be accomplished by refactoring doReport and changing printMessageAndPos to messageAndPos returning a String.

Thus you've eliminated the need for maintaining two reporters and improvements made to ConsoleReporter will carry over to the sbt reporter 👍

[felixmulder]

Since we have little to no control over the sbt-API, it is possible that you'll need to for instance give sbt an empty position so that you can reuse the one that the ConsoleReporter gives you - but that will reveal itself during the implementation :) happy coding!

[slothspot]

@felixmulder thanks for your feedback and suggestions.
I have same things on my mind and started hack on ConsoleReporter already.
Need to figure out how sbt handles a location and new lines at the end of a log message, through.

[slothspot]

@felixmulder I've updated PR, now output is like this:

Without -explain:

With -explain:

[felixmulder]

Wow looks awesome! :)

I'd rather have the methods you moved be a part of the ConsoleReporter class since that is then extensible by other people - who probably have different usecases than you and me. Could you move them back please? (to solve the issue of being able to call it's functions I'd put a private val consoleReporter = new ConsoleReporter() in the DelegatingReporter)

Otherwise, this looks great and I think we should merge it ASAP!

[felixmulder]

Or you know what - I see the point of moving them out into a separate object. Keep them separate - but make it a trait that you then mix into both the ConsoleReporter and DelegatingReporter. That way, you keep the rendering logic separate from the entity actually doing the printing.

I'd call the trait something like MessageRendering so that it becomes:

class ConsoleReporter(...) extends Reporter with ... with MessageRendering { .. }

[felixmulder]

So this guy becomes trait MessageRendering

[felixmulder]

Space after :

[felixmulder]

Weird indentation here, ident under the | signs

[felixmulder]

Space after : here as well

[slothspot]

Done

[felixmulder]

Great job! We really appreciate this PR as it greatly improves the experience when using the sbt-dotty plugin or the dotty-example-project 🎉