Support doc task with dottydoc
#4952
Conversation
|
This is only about supporting running |
| dependencyResolution.update(descriptor, updateConfiguration, warningConfiguration, log) match { | ||
| case Left(uw) => | ||
| throw new MessageOnlyException( | ||
| s"Couldn't retrieve `${scalaOrganization.value} %% $moduleName %% {scalaVersion.value}`.") |
There was a problem hiding this comment.
Missing $ before {scalaVersion.value}?
| @@ -1,4 +1,5 @@ | |||
| > run | |||
| > doc | |||
| > 'set initialCommands := "1 + 1" ' | |||
| # FIXME: does not work on the CI | |||
There was a problem hiding this comment.
What's preventing this from being run on the CI? It would be nice to also check that this generates what it's supposed to.
There was a problem hiding this comment.
The REPL doesn't work in the CI. JLine doesn't work outside of a "real terminal"
There was a problem hiding this comment.
There might be a way via some JLine option to support dumb or virtual terminals
(https://github.com/jline/jline3/wiki/Terminals) or sth. else... do we have an issue to investigate?
There was a problem hiding this comment.
By default JLine spawns a "dumb" terminal if not able to create a terminal. But you can't do anything with it, not sure we can even use it for test purpose. So i disabled it:
https://github.com/lampepfl/dotty/blob/0d7826c8a77bfa0da727ac21213475739faa9428/compiler/src/dotty/tools/repl/JLineTerminal.scala#L21
project/Build.scala
Outdated
| @@ -281,9 +281,12 @@ object Build { | |||
| } | |||
|
|
|||
| // All compiler dependencies except the library | |||
There was a problem hiding this comment.
Comment needs to be updated?
project/Build.scala
Outdated
| .map(_.data) | ||
| val otherDependencies = { | ||
| val excluded = Set("dotty-library", "dotty-compiler") | ||
| fullClasspath.in(`dotty-doc`, Compile).value |
There was a problem hiding this comment.
We're still interested in the compiler dependencies. Does this work because dotty-doc depends on dotty-compiler? Maybe add a comment
| @@ -1,4 +1,5 @@ | |||
| > run | |||
| > doc | |||
There was a problem hiding this comment.
This is not run on PRs. Did you run it locally? I'll take your words for it 😄
There was a problem hiding this comment.
Works fine on my machine at least.
| } | ||
| }, | ||
|
|
||
| scalaInstance := Def.taskDyn { |
There was a problem hiding this comment.
Why is this a taskDyn? Is it to only evaluate fetchArtifactsOf if isDotty is true?
| @@ -2,8 +2,11 @@ package dotty.tools.sbtplugin | |||
|
|
|||
| import sbt._ | |||
| import sbt.Keys._ | |||
| import sbt.librarymanagement.DependencyResolution | |||
| import sbt.internal.inc.ScalaInstance | |||
| // import sbt.inc.{ ClassfileManager, IncOptions } | |||
There was a problem hiding this comment.
Can you remove this while you're at it
| ) | ||
| } | ||
|
|
||
| private def fetchArtifactsOf(moduleName: String) = Def.task { |
There was a problem hiding this comment.
Can you add a comment:
/** Fetch artefacts for scalaOrganization.value %% moduleName % scalaVersion.value */
Duhemm
force-pushed
the
topic/sbt-support-dottydoc
branch
from
17db5ac to
d52071f
Compare
| val descriptor = dependencyResolution.wrapDependencyInModule(moduleID, scalaInfo) | ||
|
|
||
| dependencyResolution.update(descriptor, updateConfiguration, warningConfiguration, log) match { | ||
| case Left(uw) => |
There was a problem hiding this comment.
Nitpick: since uw is unused I would rewrite as:
... match {
case Right(report) =>
report.allFiles
case _ =>
throw new MessageOnlyException(...
Duhemm
force-pushed
the
topic/sbt-support-dottydoc
branch
from
d52071f to
288d9b4
Compare
Fix #4965.