Skip to content

REPL pretty-printed output does not distinguish between escape sequences and escaped escape sequences #3788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
smarter opened this issue Jan 10, 2018 · 6 comments

Comments

@smarter
Copy link
Member

smarter commented Jan 10, 2018

These strings are different:

scala> "\n" 
val res0: String = "\n"
scala> "\\n" 
val res1: String = "\n"
@jendrikw
Copy link

jendrikw commented Mar 9, 2018

I think this could be fixed by simply adding the backslash as a special case to be escaped in library/src/dotty/Show.scala for stringShow and charShow.

@Blaisorblade
Copy link
Contributor

@jendrikw Makes sense, wanna try?
The duplication between stringShow and charShow can probably also be refactored using in stringShow

str.foreach { sb.append(c.show) }

@jendrikw
Copy link

jendrikw commented Mar 9, 2018

I'll do it this weekend. Is there a reason this is not written as map and mkString?

@Blaisorblade
Copy link
Contributor

That's from e4f7b38; I'm not sure whether unspecific fears about map performance are relevant, and I'm still hoping that inlining would fix any issue, but maybe @smarter knows? Or using CI's performance testing (by writing "test performance please", like for instance #4084 (comment)), could help being sure, but I'm not 100% sure how it works 😅

@smarter
Copy link
Member Author

smarter commented Mar 9, 2018

Dotty isn't going to inline anything compiled by Scala 2, including the standard library. Running our standard benchmarks won't help you here since we don't benchmark the REPL. In any case, I doubt this is going to make any perceivable difference in performance.

@Blaisorblade
Copy link
Contributor

In any case, I doubt this is going to make any perceivable difference in performance.

That's what I hoped, thanks! So @jendrikw go ahead!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants