Use color for showing megaphases #14394
Conversation
|
in the grand tradition of scala/bug#11548 (comment) |
|
|
|
We could get by with yellow, cyan, magenta. |
|
Should also check what happens if the background is white. Some of the colors might be hard to see. |
|
skinny window wrapping as for option help:
white background in WSL terminal window, where the gray is foreground
|
| @@ -134,6 +134,7 @@ private sealed trait VerboseSettings: | |||
| self: SettingGroup => | |||
| val Vhelp: Setting[Boolean] = BooleanSetting("-V", "Print a synopsis of verbose options.") | |||
| val Xprint: Setting[List[String]] = PhasesSetting("-Vprint", "Print out program after", aliases = List("-Xprint")) | |||
| val XshowPhases: Setting[Boolean] = BooleanSetting("-Vphases", "List compiler phases.", aliases = List("-Xshow-phases")) | |||
There was a problem hiding this comment.
Why was this change made?
There was a problem hiding this comment.
That is alignment with scala 2.
There was a problem hiding this comment.
Alias means -Xshow-phases is still supported. I've considered it would be nice if help text showed aliases, perhaps under -verbose -help.
| @@ -191,3 +144,56 @@ trait CliCommand: | |||
|
|
|||
| extension [T](setting: Setting[T]) | |||
| protected def value(using ss: SettingsState): T = setting.valueIn(ss) | |||
|
|
|||
| extension (s: String) | |||
| def padLeft(width: Int): String = StringBuilder().tap(_.append(" " * (width - s.length)).append(s)).toString | |||
There was a problem hiding this comment.
| def padLeft(width: Int): String = StringBuilder().tap(_.append(" " * (width - s.length)).append(s)).toString | |
| def padLeft(width: Int): String = String.format("%1$" + width "s", s) |
There was a problem hiding this comment.
I have been preferring "just emit the string" to "parse a format template then construct a string" because people complain about slow strings. Not sure if I would push back here because format is easier to read.
There was a problem hiding this comment.
format is easier to read.
This. Code is read more often than writ.
We need ability to create strings lazily and efficiently. Also f"%${width}s" could actually detect that the interpolated value is part of the format and translate to padding, etc.
There was a problem hiding this comment.
Otherwise we could simply have
def padLeft(width: Int): String = StringBuilder().tap(_.append(" " * (width - s.length)).append(s)).toString
def padLeft(width: Int): String = " " * (width - s.length) + sThere is a single String.+, the builder will not help for this use case.
| val field1 = maxField.min(texts.flatten.map(_._1.length).filter(_ < maxField).max) // widest field under maxField | ||
| val field2 = if field1 + separation + maxField < maxCol then maxCol - field1 - separation else 0 // skinny window -> terminal wrap | ||
| val separator = " " * separation | ||
| def formatField1(text: String): String = if text.length <= field1 then text.padLeft(field1) else text + EOL + "".padLeft(field1) |
There was a problem hiding this comment.
| def formatField1(text: String): String = if text.length <= field1 then text.padLeft(field1) else text + EOL + "".padLeft(field1) | |
| def formatField1(text: String): String = if text.length <= field1 then text.padLeft(field1) else text + "\n" + "".padLeft(field1) |
We had some issues with EOL and Windows in the past. The "\n" seems to be what we should use by default.
There was a problem hiding this comment.
I have followed the issue, but usually if I'm on cygwin I want the correct line separator. I'll try to come up with a use case slash test.
There was a problem hiding this comment.
I'll try to come up with a use case slash test.
Maybe if I start using cygwin again on a regular basis...
|
I guess he didn't add a test because it's too hard? |
som-snytt
force-pushed
the
tweak/color-phases
branch
from
23be905 to
c61aefd
Compare
som-snytt
force-pushed
the
tweak/color-phases
branch
from
c61aefd to
12e98f5
Compare
|
Addressed comments, re-rebased against main instead of master. I have to say, those colors are pretty. |
Tweak to #14388