Skip to content

Commit

Permalink
Don't force LF line endings on Windows
Browse files Browse the repository at this point in the history
  • Loading branch information
alexarchambault committed Jun 3, 2024
1 parent 3f79a8b commit 415e54c
Show file tree
Hide file tree
Showing 18 changed files with 226 additions and 100 deletions.
20 changes: 0 additions & 20 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
107 changes: 100 additions & 7 deletions modules/scala/examples/src/test/scala/almond/examples/Examples.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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,
Expand All @@ -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": {
Expand All @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ final class ScalaInterpreter(
)
}

private def nl = System.lineSeparator()
def kernelInfo() =
KernelInfo(
"scala",
Expand All @@ -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)
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, "", "...")
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
31 changes: 28 additions & 3 deletions modules/scala/scala-kernel/src/main/scala/almond/Scalafmt.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 415e54c

Please sign in to comment.