Don't force LF line endings on Windows (#1089)

.github/workflows/ci.yml

@@ -15,11 +15,6 @@ jobs:
       matrix:
         OS: ["ubuntu-latest", "windows-latest"]
     steps:
-    - name: Don't convert LF to CRLF during checkout
-      if: runner.os == 'Windows'
-      run: |
-        git config --global core.autocrlf false
-        git config --global core.eol lf
     - uses: actions/checkout@v4
       with:
         fetch-depth: 0
@@ -57,11 +52,6 @@ jobs:
             JDK: 17
             SCALA: 2.12.19
     steps:
-    - name: Don't convert LF to CRLF during checkout
-      if: runner.os == 'Windows'
-      run: |
-        git config --global core.autocrlf false
-        git config --global core.eol lf
     - uses: actions/checkout@v4
       with:
         fetch-depth: 0
@@ -83,11 +73,6 @@ jobs:
       matrix:
         OS: ["ubuntu-22.04", macos-12, windows-latest]
     steps:
-    - name: Don't convert LF to CRLF during checkout
-      if: runner.os == 'Windows'
-      run: |
-        git config --global core.autocrlf false
-        git config --global core.eol lf
     - uses: actions/checkout@v4
       with:
         fetch-depth: 0
@@ -102,11 +87,6 @@ jobs:
   bincompat:
     runs-on: ubuntu-latest
     steps:
-    - name: Don't convert LF to CRLF during checkout
-      if: runner.os == 'Windows'
-      run: |
-        git config --global core.autocrlf false
-        git config --global core.eol lf
     - uses: actions/checkout@v4
       with:
         fetch-depth: 0

modules/scala/examples/src/test/scala/almond/examples/Examples.scala

@@ -43,9 +43,89 @@ class Examples extends munit.FunSuite {
     dir
   }
 
+  lazy val inputDir = outputDir / "input"
+
+  private val nl = System.lineSeparator()
+  private lazy val escapedNl =
+    if (nl == "\n") "\\n"
+    else if (nl == "\r\n") "\\r\\n"
+    else ???
+  private val shouldUpdateLineSep = System.lineSeparator() == "\r\n"
+
+  private def traverseAndUpdateLineSep(content: ujson.Value, deep: Boolean = false): Option[ujson.Value] =
+    content.arrOpt match {
+      case Some(arr) =>
+        for ((elem, idx) <- arr.zipWithIndex)
+          for (updatedElem <- traverseAndUpdateLineSep(elem, deep = deep))
+            content(idx) = updatedElem
+        None
+      case None =>
+        content.objOpt match {
+          case Some(obj) =>
+            for ((k, v) <- obj)
+              for (updatedElem <- traverseAndUpdateLineSep(v, deep = deep))
+                content(k) = updatedElem
+            None
+          case None =>
+            content.strOpt.map { str =>
+              if (deep)
+                str
+                  .linesWithSeparators
+                  .map { line =>
+                    if (deep)
+                      line.replace("\\n", escapedNl)
+                    else
+                      line
+                  }
+                  .mkString
+              else
+                str
+                  .linesWithSeparators
+                  .flatMap { line =>
+                    if (line.endsWith("\n") && !line.endsWith(nl))
+                      Iterator(line.stripSuffix("\n"), nl)
+                    else
+                      Iterator(line)
+                  }
+                  .mkString
+            }
+        }
+    }
+
+  private def updateLineSep(content: String): String = {
+
+    val json = ujson.read(content)
+    for (cell <- json("cells").arr if cell("cell_type").str == "code") {
+      if (cell.obj.contains("outputs"))
+        for (output <- cell("outputs").arr if output("output_type").strOpt.exists(s => s == "display_data" || s == "execute_result"))
+          for ((k, v) <- output("data").obj) {
+            val shouldUpdate =
+              (k.startsWith("text/") && !v.arr.exists(_.str.contains("function(Plotly)"))) ||
+                k == "application/vnd.plotly.v1+json"
+            if (shouldUpdate)
+              traverseAndUpdateLineSep(v)
+            if ((k == "text/html" && v.arr.exists(_.str.contains("function(Plotly)"))))
+              traverseAndUpdateLineSep(v, deep = true)
+          }
+      if (cell.obj.contains("source"))
+        traverseAndUpdateLineSep(cell("source"))
+    }
+
+    json.render(1).replace("\n", nl)
+  }
+
   for (notebook <- notebooks)
     test(notebook.last.stripSuffix(".ipynb")) {
 
+      val input =
+        if (shouldUpdateLineSep) {
+          val dest = inputDir / notebook.last
+          val updatedContent = updateLineSep(os.read(notebook))
+          os.write.over(dest, updatedContent, createFolders = true)
+          dest
+        }
+        else
+          notebook
       val output = outputDir / notebook.last
       val res = os.proc(
         "jupyter",
@@ -54,7 +134,7 @@ class Examples extends munit.FunSuite {
         "notebook",
         "--execute",
         s"--ExecutePreprocessor.kernel_name=$kernelId",
-        notebook,
+        input,
         s"--output=$output"
       ).call(
         cwd = ExampleProperties.directory,
@@ -72,8 +152,6 @@ class Examples extends munit.FunSuite {
       val rawOutput = os.read(output, Charset.defaultCharset())
 
       var updatedOutput = rawOutput
-      if (Properties.isWin)
-        updatedOutput = updatedOutput.replace("\r\n", "\n").replace("\\r\\n", "\\n")
 
       // Clear metadata, that usually looks like
       // "metadata": {
@@ -89,19 +167,34 @@ class Examples extends munit.FunSuite {
         cell("metadata") = ujson.Obj()
       updatedOutput = json.render(1)
 
+      if (Properties.isWin)
+        updatedOutput = updatedOutput.replace("\n", "\r\n")
+
+      val result = updatedOutput
+
       // writing the updated notebook on disk for the diff below
-      os.write.over(output, updatedOutput.getBytes(Charset.defaultCharset()))
+      os.write.over(output, result.getBytes(Charset.defaultCharset()))
 
-      val result   = os.read(output, Charset.defaultCharset())
-      val expected = os.read(notebook)
+      val expected = os.read(input)
 
       if (result != expected) {
+        def explicitCrLf(input: String): String =
+          input
+            .replace("\r", "\\r\r")
+            .replace("\n", "\\n\n")
+            .replace("\\r\r\\n\n", "\\r\\n\r\n")
         System.err.println(s"${notebook.last} differs:")
+        System.err.println(s"Expected ${expected.length} chars, got ${result.length}")
         System.err.println()
-        os.proc("diff", "-u", notebook, output)
+        os.proc("diff", "-u", input, output)
           .call(cwd = ExampleProperties.directory, check = false, stdin = os.Inherit, stdout = os.Inherit)
         if (update) {
           System.err.println(s"Updating ${notebook.last}")
+          if (shouldUpdateLineSep)
+            System.err.println(
+              "Warning: the current system uses CRLF as line separator, " +
+              "only notebooks using LF as line separator should be committed."
+            )
           os.copy.over(output, notebook)
         }
         sys.error("Output notebook differs from original")

modules/scala/scala-interpreter/src/main/scala-2/almond/internals/AlmondMetabrowseServer.scala

@@ -93,12 +93,14 @@ final class AlmondMetabrowseServer(
     val sourcePath = AlmondMetabrowseServer.sourcePath(frames, log)
 
     log.info(s"Starting metabrowse server at http://$metabrowseHost:$port")
-    log.info(
-      "Initial source path\n  Classpath\n" +
-        sourcePath.classpath.map("    " + _).mkString("\n") +
-        "\n  Sources\n" +
-        sourcePath.sources.map("    " + _).mkString("\n")
-    )
+    log.info {
+      val nl = System.lineSeparator()
+      "Initial source path" + nl +
+        "  Classpath" + nl +
+        sourcePath.classpath.map("    " + _).mkString(nl) + nl +
+        "  Sources" + nl +
+        sourcePath.sources.map("    " + _).mkString(nl)
+    }
     server.start(sourcePath)
 
     (server, port, windowName)
@@ -181,11 +183,11 @@ object AlmondMetabrowseServer {
       .map(Paths.get)
       .toList
 
-    log.info(
-      "Found base JARs:\n" +
-        baseJars.sortBy(_.toString).map("  " + _).mkString("\n") +
-        "\n"
-    )
+    log.info {
+      val nl = System.lineSeparator()
+      "Found base JARs:" + nl +
+        baseJars.sortBy(_.toString).map("  " + _).mkString(nl) + nl
+    }
 
     // When using a "hybrid" launcher, and users decided to end its name with ".jar",
     // we still want to use it as a source JAR too. So we check if it contains sources here.

modules/scala/scala-interpreter/src/main/scala/almond/Execute.scala

@@ -256,9 +256,10 @@ final class Execute(
             log.warn("Received SIGINT, but no execution is running")
           case Some(t) =>
             interruptedStackTraceOpt0 = Some(t.getStackTrace)
-            log.debug(
-              s"Received SIGINT, stopping thread $t\n${interruptedStackTraceOpt0.map("  " + _).mkString("\n")}"
-            )
+            log.debug {
+              val nl = System.lineSeparator()
+              s"Received SIGINT, stopping thread $t$nl${interruptedStackTraceOpt0.map("  " + _).mkString(nl)}"
+            }
             if (useThreadInterrupt) {
               log.debug(s"Calling 'Thread.interrupt'")
               t.interrupt()
@@ -283,9 +284,10 @@ final class Execute(
       case None =>
         log.warn("Interrupt asked, but no execution is running")
       case Some(t) =>
-        log.debug(
-          s"Interrupt asked, stopping thread $t\n${t.getStackTrace.map("  " + _).mkString("\n")}"
-        )
+        log.debug {
+          val nl = System.lineSeparator()
+          s"Interrupt asked, stopping thread $t$nl${t.getStackTrace.map("  " + _).mkString(nl)}"
+        }
         if (useThreadInterrupt) {
           log.debug(s"Calling 'Thread.interrupt'")
           t.interrupt()

modules/scala/scala-interpreter/src/main/scala/almond/ReplApiImpl.scala

@@ -144,7 +144,8 @@ final class ReplApiImpl(
                         Attr.Reset,
                         colors0().literal()
                       )
-                    val s = messageColor("[last attempt failed]").render + "\n" + err
+                    val s =
+                      messageColor("[last attempt failed]").render + System.lineSeparator() + err
                     updatableResults.update(id, s, last = false)
                   case Right(value0) =>
                     val s = pprinter().tokenize(
@@ -207,6 +208,25 @@ final class ReplApiImpl(
 
       val defaultDisplayer = Displayers.registration().find(classOf[ReplApiImpl.Foo])
 
+      private val shouldUpdateLineSep = System.lineSeparator() != "\n"
+      private val nl                  = System.lineSeparator()
+      override def combinePrints(iters: Iterator[String]*): Iterator[String] =
+        super.combinePrints((
+          if (shouldUpdateLineSep)
+            // these iterators mostly contain strings generated by PPrint,
+            // which uses solely "\n" as line ending
+            iters.map(_.flatMap { elem =>
+              elem.linesWithSeparators.flatMap { line =>
+                if (line.endsWith("\n") && !line.endsWith(nl))
+                  Iterator(line.stripSuffix("\n"), nl)
+                else
+                  Iterator(line)
+              }
+            })
+          else
+            iters
+        ): _*)
+
       override def print[T](
         value: => T,
         ident: String,

modules/scala/scala-interpreter/src/main/scala/almond/ScalaInterpreter.scala

@@ -285,6 +285,7 @@ final class ScalaInterpreter(
     )
   }
 
+  private def nl = System.lineSeparator()
   def kernelInfo() =
     KernelInfo(
       "scala",
@@ -301,7 +302,7 @@ final class ScalaInterpreter(
          |Ammonite ${ammonite.Constants.version}
          |${scala.util.Properties.versionMsg}
          |Java ${sys.props.getOrElse("java.version", "[unknown]")}""".stripMargin +
-        params.extraBannerOpt.fold("")("\n\n" + _),
+        params.extraBannerOpt.fold("")(nl + nl + _),
       help_links = Some(params.extraLinks.toList).filter(_.nonEmpty)
     )
 

modules/scala/scala-interpreter/src/main/scala/almond/VariableInspectorApiImpl.scala

@@ -106,9 +106,9 @@ private object VariableInspectorApiImpl {
                 )
                 .map(_.render)
                 .mkString
-                .replaceAll(java.util.regex.Pattern.quote("(") + "\n\\s+", "(")
-                .replaceAll("\n\\s+", " ")
-                .replaceAll("\n", " ")
+                .replaceAll(java.util.regex.Pattern.quote("(") + "\r?\n\\s+", "(")
+                .replaceAll("\r?\n\\s+", " ")
+                .replaceAll("\r?\n", " ")
           },
         varType = tprint.render(TPrintColors.BlackWhite).render,
         isMatrix = false,

modules/scala/scala-interpreter/src/test/scala/almond/ScalaKernelTests.scala

@@ -763,8 +763,8 @@ object ScalaKernelTests extends TestSuite {
         "res2: Vector[Int] = " +
           (1 to 38)
             .toVector
-            .map("  " + _ + "," + "\n")
-            .mkString("Vector(" + "\n", "", "...")
+            .map("  " + _ + "," + nl)
+            .mkString("Vector(" + nl, "", "...")
       )
     }
 

modules/scala/scala-kernel/src/main/scala/almond/ScalaKernel.scala

@@ -115,7 +115,7 @@ object ScalaKernel extends CaseApp[Options] {
             case (trigger, auto) =>
               Seq("Auto dependency:", s"  Trigger: $trigger") ++ auto.map(dep => s"  Adds: $dep")
           }
-          .mkString("\n")
+          .mkString(System.lineSeparator())
       )
 
     val interpreterEc               = singleThreadedExecutionContext("scala-interpreter")

modules/scala/scala-kernel/src/main/scala/almond/Scalafmt.scala

@@ -49,10 +49,35 @@ final class Scalafmt(
       s"runner.dialect=$defaultDialect"
     ).map(_ + System.lineSeparator).mkString
 
-  private def format(code: String): String =
+  private def usesCrlf(code: String): Boolean = {
+    var hasLines = false
+    val onlyCrlf = code
+      .linesWithSeparators
+      .forall { line =>
+        hasLines = true
+        line.endsWith("\r\n")
+      }
+    hasLines && onlyCrlf
+  }
+
+  private def format(code: String): String = {
     // TODO Get version via build.sbt
-    interface.format(confFile(defaultConfFile), defaultDummyPath, code)
-      .stripSuffix("\n") // System.lineSeparator() instead?
+    val rawResult = interface.format(confFile(defaultConfFile), defaultDummyPath, code)
+    // Seems scalafmt discards crlf line endings
+    if (usesCrlf(code))
+      rawResult
+        .linesWithSeparators
+        .flatMap { line =>
+          if (line.endsWith("\n") && !line.endsWith("\r\n"))
+            Iterator(line.stripSuffix("\n"), "\r\n")
+          else
+            Iterator(line)
+        }
+        .mkString
+        .stripSuffix("\r\n")
+    else
+      rawResult.stripSuffix("\n")
+  }
 
   def messageHandler: MessageHandler =
     MessageHandler.blocking(Channel.Requests, Format.requestType, queueEc, logCtx) {