Skip to content

Commit

Permalink
Replace full-width prompt header with a variable-width-with-max promp…
Browse files Browse the repository at this point in the history 
…t header (#4014)

The previous full-width prompt header looks really weird on wide
screens, even a full-width laptop screen is enough to make it awkward.
This PR replaces it with a variable-width prompt header with some
heuristics for shortening the `=` bars and the title text as necessary.

Covered by existing tests, added some more thorough tests in
`PromptLoggerUtilTests.scala` to exercise the edge cases in the new
shortening heuristics
  • Loading branch information
@lihaoyi
lihaoyi authored Nov 24, 2024
1 parent 472c5f0 commit 5dbfd67
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 110 deletions.
37 changes: 19 additions & 18 deletions integration/feature/full-run-logs/src/FullRunLogsTests.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,25 @@ object FullRunLogsTests extends UtestIntegrationTestSuite {
res.isSuccess ==> true
assert("\\[\\d+\\] <h1>hello</h1>".r.matches(res.out))

val expectedErrorRegex =
s"""========================================== run --text hello ======================================
|==================================================================================================
|[build.mill-<digits>/<digits>] compile
|[build.mill-<digits>] [info] compiling 1 Scala source to ${tester.workspacePath}/out/mill-build/compile.dest/classes ...
|[build.mill-<digits>] [info] done compiling
|[<digits>/<digits>] compile
|[<digits>] [info] compiling 1 Java source to ${tester.workspacePath}/out/compile.dest/classes ...
|[<digits>] [info] done compiling
|[<digits>/<digits>] run
|[<digits>/<digits>] ================================== run --text hello ==================================<digits>s
|=================================================================================================="""
.stripMargin
.replaceAll("(\r\n)|\r", "\n")
.replace('\\', '/')
.split("<digits>")
.map(java.util.regex.Pattern.quote)
.mkString("=? ?[\\d]+")
val expectedErrorRegex = java.util.regex.Pattern
.quote(
s"""<dashes> run --text hello <dashes>
|<dashes>
|[build.mill-<digits>/<digits>] compile
|[build.mill-<digits>] [info] compiling 1 Scala source to ${tester.workspacePath}/out/mill-build/compile.dest/classes ...
|[build.mill-<digits>] [info] done compiling
|[<digits>/<digits>] compile
|[<digits>] [info] compiling 1 Java source to ${tester.workspacePath}/out/compile.dest/classes ...
|[<digits>] [info] done compiling
|[<digits>/<digits>] run
|[<digits>/<digits>] <dashes> run --text hello <dashes> <digits>s
|<dashes>"""
.stripMargin
.replaceAll("(\r\n)|\r", "\n")
.replace('\\', '/')
)
.replace("<digits>", "\\E\\d+\\Q")
.replace("<dashes>", "\\E=+\\Q")

assert(expectedErrorRegex.r.matches(res.err.replace('\\', '/').replaceAll("(\r\n)|\r", "\n")))
}
Expand Down
34 changes: 15 additions & 19 deletions main/util/src/mill/util/PromptLoggerUtil.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ private object PromptLoggerUtil {
// For non-interactive jobs, the prompt won't be at the bottom of the terminal but
// will instead be in the middle of a big log file with logs above and below, so we
// need some kind of footer to tell the reader when the prompt ends and logs begin
val footer = Option.when(!interactive)("=" * maxWidth).toList
val footer = Option.when(!interactive)("=" * header.length).toList

header :: body ::: footer
}
Expand Down Expand Up @@ -187,28 +187,24 @@ private object PromptLoggerUtil {
val headerPrefixStr = if (!interactive || ending) headerPrefix else s" $headerPrefix"
val headerSuffixStr = headerSuffix0
val titleText = s" $titleText0 "
// -12 just to ensure we always have some ==== divider on each side of the title

val dividerMaxLength = 30
val dividerMinLength = 15
val maxTitleLength =
maxWidth - math.max(headerPrefixStr.length, headerSuffixStr.length) * 2 - 12
maxWidth - headerPrefixStr.length - headerSuffixStr.length - dividerMinLength * 2
val shortenedTitle = splitShorten(titleText, maxTitleLength)

// +2 to offset the title a bit to the right so it looks centered, as the `headerPrefixStr`
// is usually longer than `headerSuffixStr`. We use a fixed offset rather than dynamically
// offsetting by `headerPrefixStr.length` to prevent the title from shifting left and right
// as the `headerPrefixStr` changes, even at the expense of it not being perfectly centered.
val leftDivider = "=" * ((maxWidth / 2) - (titleText.length / 2) - headerPrefixStr.length + 2)
val rightDivider =
"=" * (
maxWidth - headerPrefixStr.length - leftDivider.length -
shortenedTitle.length - headerSuffixStr.length
)
val headerString =
headerPrefixStr + leftDivider + shortenedTitle + rightDivider + headerSuffixStr
assert(
headerString.length == maxWidth,
s"${pprint.apply(headerString)} is length ${headerString.length}, requires $maxWidth"
val rightDiv = "=" * math.min(
dividerMaxLength,
(maxWidth - headerPrefixStr.length - headerSuffixStr.length - shortenedTitle.length) / 2
)
headerString
val leftDiv = "=" * math.min(
dividerMaxLength,
maxWidth - headerPrefixStr.length - headerSuffixStr.length - shortenedTitle.length - rightDiv.length
)

val headerString = headerPrefixStr + leftDiv + shortenedTitle + rightDiv + headerSuffixStr
splitShorten(headerString, maxWidth)
}

def splitShorten(s: String, maxLength: Int): String = {
Expand Down
40 changes: 20 additions & 20 deletions main/util/test/src/mill/util/PromptLoggerTests.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ object PromptLoggerTests extends TestSuite {
promptLogger.close()

check(promptLogger, baos, width = 999 /*log file has no line wrapping*/ )(
"================================================ TITLE ===========================================",
"==================================================================================================",
"============================== TITLE ==============================",
"===================================================================",
// Make sure that the first time a prefix is reported,
// we print the verbose prefix along with the ticker string
"[1/456] my-task",
Expand All @@ -89,17 +89,17 @@ object PromptLoggerTests extends TestSuite {
// the double space prefix (since it's non-interactive and we don't need space for a cursor),
// the time elapsed, the reported title and ticker, the list of active tickers, followed by the
// footer
"[123/456] ====================================== TITLE ======================================= 10s",
"[123/456] ============================== TITLE ============================== 10s",
"[1] my-task 10s",
"==================================================================================================",
"=================================================================================",
"[1] WORLD",
// Calling `refreshPrompt()` after closing the ticker shows the prompt without
// the ticker in the list, with an updated time elapsed
"[123/456] ====================================== TITLE ======================================= 20s",
"==================================================================================================",
"[123/456] ============================== TITLE ============================== 20s",
"=================================================================================",
// Closing the prompt prints the prompt one last time with an updated time elapsed
"[123/456] ====================================== TITLE ======================================= 30s",
"==================================================================================================",
"[123/456] ============================== TITLE ============================== 30s",
"=================================================================================",
""
)
}
Expand All @@ -113,7 +113,7 @@ object PromptLoggerTests extends TestSuite {
promptLogger.setPromptHeaderPrefix("123/456")
promptLogger.refreshPrompt()
check(promptLogger, baos)(
" [123/456] ========================== TITLE =================================="
" [123/456] ============================== TITLE =============================="
)
promptLogger.setPromptLine(Seq("1"), "/456", "my-task")

Expand All @@ -130,7 +130,7 @@ object PromptLoggerTests extends TestSuite {
"",
"[1/456] my-task",
"[1] HELLO",
" [123/456] ========================== TITLE ============================== 10s",
" [123/456] ============================ TITLE ============================ 10s",
"[1] my-task 10s"
)

Expand All @@ -143,7 +143,7 @@ object PromptLoggerTests extends TestSuite {
"[1/456] my-task",
"[1] HELLO",
"[1] WORLD",
" [123/456] ========================== TITLE ============================== 10s",
" [123/456] ============================ TITLE ============================ 10s",
"[1] my-task 10s"
)

Expand Down Expand Up @@ -175,7 +175,7 @@ object PromptLoggerTests extends TestSuite {
"[3/456] my-task-short-lived",
"[3] hello short lived",
"[3] goodbye short lived",
" [123/456] ========================== TITLE ============================== 10s",
" [123/456] ============================ TITLE ============================ 10s",
"[1] my-task 10s"
)

Expand All @@ -196,7 +196,7 @@ object PromptLoggerTests extends TestSuite {
"[3/456] my-task-short-lived",
"[3] hello short lived",
"[3] goodbye short lived",
" [123/456] ========================== TITLE ============================== 11s",
" [123/456] ============================ TITLE ============================ 11s",
"[1] my-task 11s",
"[2] my-task-new 1s"
)
Expand All @@ -218,7 +218,7 @@ object PromptLoggerTests extends TestSuite {
"[3/456] my-task-short-lived",
"[3] hello short lived",
"[3] goodbye short lived",
" [123/456] ========================== TITLE ============================== 11s",
" [123/456] ============================ TITLE ============================ 11s",
"[1] my-task 11s",
"[2] my-task-new 1s"
)
Expand All @@ -239,7 +239,7 @@ object PromptLoggerTests extends TestSuite {
"[3/456] my-task-short-lived",
"[3] hello short lived",
"[3] goodbye short lived",
" [123/456] ========================== TITLE ============================== 12s",
" [123/456] ============================ TITLE ============================ 12s",
"[2] my-task-new 2s",
""
)
Expand All @@ -259,7 +259,7 @@ object PromptLoggerTests extends TestSuite {
"[3/456] my-task-short-lived",
"[3] hello short lived",
"[3] goodbye short lived",
" [123/456] ========================== TITLE ============================== 22s",
" [123/456] ============================ TITLE ============================ 22s",
"[2] my-task-new 12s"
)
now += 10000
Expand All @@ -275,7 +275,7 @@ object PromptLoggerTests extends TestSuite {
"[3/456] my-task-short-lived",
"[3] hello short lived",
"[3] goodbye short lived",
"[123/456] ============================ TITLE ============================== 32s",
"[123/456] ============================= TITLE ============================= 32s",
""
)
}
Expand All @@ -298,20 +298,20 @@ object PromptLoggerTests extends TestSuite {
now += 1000
promptLogger.refreshPrompt()
check(promptLogger, baos)(
" [123/456] ========================== TITLE =============================== 1s",
" [123/456] ============================= TITLE ============================ 1s",
"[1] my-task 1s detail"
)
prefixLogger.ticker("detail-too-long-gets-truncated-abcdefghijklmnopqrstuvwxyz1234567890")
promptLogger.refreshPrompt()
check(promptLogger, baos)(
" [123/456] ========================== TITLE =============================== 1s",
" [123/456] ============================= TITLE ============================ 1s",
"[1] my-task 1s detail-too-long-gets-truncated...fghijklmnopqrstuvwxyz1234567890"
)
promptLogger.removePromptLine(Seq("1"))
now += 10000
promptLogger.refreshPrompt()
check(promptLogger, baos)(
" [123/456] ========================== TITLE ============================== 11s"
" [123/456] ============================ TITLE ============================ 11s"
)
}
}
Expand Down
Loading

0 comments on commit 5dbfd67

Please sign in to comment.