scalameta / mdoc

Typechecked markdown documentation for Scala
https://scalameta.org/mdoc/
Apache License 2.0
394 stars 81 forks source link

Update handling of end-of-line single-line comments #825

Closed julianpeeters closed 8 months ago

julianpeeters commented 8 months ago

Fix for #321, #605 🎄🎁

Adopted solution: distinguish between leading trivia a) previous trailing trivia, b) current leading trivia, c) a footer in case of last statement.

Hello,

It's really fun slowly discovering all that mdoc can do. However, I finally ran into the comment duplication issue (for me, import statements). Following the threads from previous comments (in the issues linked above), I propose the changes in this PR:

What other strategies did I consider but reject?

Happy to make changes as needed, and thanks for your consideration.

julianpeeters commented 8 months ago

outdated

I could use some help interpreting the failures, however.

On one hand, testing locally with sbt unit/test I get [success]

While, on the other hand,

  • some of the test martix succeeds
  • but, others of the test matrix seem to print differently than the others?

Specifically, this expectation seems to introduce a problem having to do with how the variable // a: Some[Int] = Some(1) is rendered:

==> X tests.markdown.VariablePrinterSuite.single-line-comment  0.722s munit.ComparisonFailException: /home/runner/work/mdoc/mdoc/tests/unit/src/test/scala/tests/markdown/VariablePrinterSuite.scala:6
5:
6:  check(
7:    "single-line-comment",
diff assertion failed
=> Obtained
    """|```scala
       |import scala.Some // an import statement
       |val a = Some(1) // a variable
       |// a: Some[Int] = Some(1)
       |val b = 2 // another variable
       |// b: Int = 2
       |```
       |""".stripMargin
=> Diff (- obtained, + expected)
...

if I fix/adjust the expectation for one in the test matrix, I believe the others in the test matrix will break, because some runs of the test matrix render |// a: Some[Int] = Some(1), while others render |// a: Some[Int] = Some(value = 1)

pls advise

julianpeeters commented 8 months ago

updated the test to avoid the difference in the printing of values

I think the remaining failures are spurious, seemingly related to a race or lock:

java.nio.channels.OverlappingFileLockException while downloading https://oss.sonatype.org/content/repositories/public/org/scala-js/scalajs-linker_2.12/1.13.0/scalajs-linker_2.12-1.13.0.pom

May I request a re-run of the failed jobs? But first, Eggnog!

julianpeeters commented 8 months ago

Aha, much better, thanks

not sure about error on Windows, but doesn't seem related to these changes:

[error] Failed: Total 216, Failed 1, Errors 0, Passed 215, Ignored 8 [error] Failed tests: [error] tests.imports.DependencySuite

maybe spurious:

java.nio.channels.OverlappingFileLockException while downloading https://oss.sonatype.org/content/repositories/public/org/scala-js/scalajs-linker_2.12/1.13.0/scalajs-linker_2.12-1.13.0.pom

happy to update as requested