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

[julianpeeters]

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:

• Update the Render.renderEvaluatedSection method to more explicitly handle these types of comments
• Add unit tests for common cases
• Add code comments in case of further enhancement or refinement

What other strategies did I consider but reject?

• Add support for comments to the stats and Range.Position systems: my (possibly incorrect) impression is that the root cause is the lack of comments in quasiquotes, and, since adding them is out of scope, that therefore some workarounds are required
• Add an explicit model of the source (with trivia) in a code fence: prefer to trial this PR, defer solidifying/polishing up case classes, optional fields, etc.

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

[julianpeeters]

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]

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!

[tgodzik]

Thanks for the contribution!

Looks like the Scala 3 test is failing, you might need to adjust depending on the scala version, there should be some samples in the tests. Let me know if you need help figuring it out.

[julianpeeters]

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

[tgodzik]

LGTM! Thanks!