Use stringOf from REPL to avoid trimming

[som-snytt]

ScalaRunTime.replStringOf adds extra newlines. Try not to
trim more than was added.

New API will not include the newlines.

Fixes #11558

[som-snytt]

With scala 2 published locally

Starting scala3 REPL...
scala> 42
val res0: Int = 42

scala> "hi there"
val res1: String = "hi there"

scala> case class Box(x: Any) { override def toString = s"\n$x\n" }
// defined case class Box

scala> Box("xxx")
val res2: Box = Box("xxx")

scala> Box("xxx").toString
val res3: String = """
xxx
"""

scala> Box(Array(1,2,3))
val res4: Box = Box(Array(1, 2, 3))

scala> case class Box(x: Any, y: Int) { override def toString = s"\n$x\n" }
// defined case class Box

scala> Box(Array(1,2,3), 42)
val res5: Box = Box(x = Array(1, 2, 3), y = 42)

scala>  

[som-snytt]

Regressed with Scala 2 updated replStringOf.

scala> Option.empty
val res1: Option[Nothing] = None()

scala> enum E { case X } ; E.X
// defined class E
val res2: E = X()

Obviously the tests will break anyway with updated replStringOf. I thoughtlessly committed those test changes.

...because I didn't have that latest commit that truncates the replStringOf. That test started failing with an off-by-one in the length test.

...because the newline is trimmed before truncate sees it.

[bjornregnell]

I just made the comment below; I hope I'm not completely off, so take it for what it is (a question from an person who does not know the underlying code base...)

[bjornregnell]

This code seems to assume that there always is a newline at the end of res? Why not check explicitly if also the last character is a newline to be cut, without just assuming that its there (to be more robust and not living on assumptions...)? Or am I missing something?

[som-snytt]

L73 checks last, L75 checks first char. I'll look again when awake.

[bjornregnell]

Yes, my bad. I was not seeing all the cases in my head. It seems to work as far as I can tell.

[bjornregnell]

So the logic is:
if non-zero or no trailing \n then leave it as is
else if only one char or no leading \n then cut last (because we now know that there is a trailing \n)
else cut both first and last (because we now know that there must be both leading and trailing \n)

[bjornregnell]

Somehow I think this is easier to grok: (but maybe it's just me)

def trimNewlinesAtStartOrEnd(res: String) =
  val len = res.length
  if len == 0 || res == "\n" then ""
  else
    val from  = if res.charAt(0) == '\n' then 1 else 0
    val until = if res.charAt(len - 1) == '\n' then len - 1 else len
    res.substring(from, until)  

Simpler to just take the special cases first IMHO.
But take whatever version you like best :)
I just had to test it :)

scala> trimNewlinesAtStartOrEnd("")
val res0: String = ""

scala> trimNewlinesAtStartOrEnd("\n")                                                                                     
val res1: String = ""

scala> trimNewlinesAtStartOrEnd("\n\n")                                                                               
val res2: String = ""

scala> trimNewlinesAtStartOrEnd("\nhello\n")                                                                                   
val res3: String = hello

scala> trimNewlinesAtStartOrEnd("\n\n\n")                                                                                   
val res4: String = "
"
scala> trimNewlinesAtStartOrEnd("\nhello")                                                                
val res5: String = hello

scala> trimNewlinesAtStartOrEnd("hello\n")                                                                    
val res6: String = hello

scala> trimNewlinesAtStartOrEnd("hello")                                                                      
val res7: String = hello

scala> trimNewlinesAtStartOrEnd("hello\nhello")                                                                      
val res8: String = hello
hello

[bjornregnell]

(BTW: Why does replStringOf (sometimes?) add extra newlines in the first place?)

[som-snytt]

It appends always for Scala 2 REPL; other question is why doesn't Scala 3 use stringOf. I suppose it prepends on multiline so the lines are all left-adjusted.

My time box expired on this tweak, so I didn't add a special test; the repl tests break easily. I agree those lines tax the short-term memory of the reviewer; if the Scala 2 PR is merged, they will be unused code.

[som-snytt]

BTW stringOf is the original method name, not an internal helper; replStringOf was split out a decade ago out of fastidiousness. So my better option here was just to use stringOf directly. I'll take a few minutes to try that out, after I knock off of my day job.

[bjornregnell]

Ok thanks for info. LGTM.

[bjornregnell]

LGTM.

[som-snytt]

I'll push a commit that uses stringOf and avoids the dread if-else.

Without the final null check in replStringOf

scala>     val tpolecat = new Object {
     |       override def toString(): String = null
     |     }
Exception in thread "main" java.lang.NullPointerException: Cannot invoke "String.length()" because "str" is null
        at dotty.tools.repl.Rendering.truncate(Rendering.scala:97)
        at dotty.tools.repl.Rendering.replStringOf(Rendering.scala:106)

The Scala 2 test case makes me think it was something @tpolecat complained about.

[bjornregnell]

Using stringOf instead seems good to me (although I do not have insight into REPL-historic complaints @tpolecat . But I am very glad to have learnt the new English word fastidious, thanks @som-snytt. That word turns out to have really many different translations to Swedish, but no single Swedish word as a direct translation - I'm not sure if it is a bug or a feature that Swedish seems less of an ambiguous language here... I choose choosy rather than faddish as a translation after traversing all synonyms until truncate at some arbitrary max size. 🥰 )

[som-snytt]

My favorite Swedish action movie is "The Fastidious and the Furious."