Skip to content
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

Ugly log output in Mill 0.12.4 #4171

Closed
lefou opened this issue Dec 22, 2024 · 8 comments · Fixed by #4173
Closed

Ugly log output in Mill 0.12.4 #4171

lefou opened this issue Dec 22, 2024 · 8 comments · Fixed by #4173
Milestone

Comments

@lefou
Copy link
Member

lefou commented Dec 22, 2024

After bumping a project from Mill 0.12.2 to 0.12.4, the log often contains parts of the progress output ruler.

Example output (excerpt): mill __.compile

[3433] [warn] /home/lefou/work/bla/bla/framework/ui.binding.rcp/src/de/bla/richmodel/ui/binding/rcp/jface/formsupport/formeditor/OldRmfEditorPart.java:186:62: getMainBinding() in de.bla.richmodel.ui.binding.editor.EditorController ist veraltet
[3433] [warn] getEditorController().getMainBinding
[3433] [warn]                                     ^
[3433] [warn] /home/lefou/work/bla/bla/framework/ui.binding.rcp/src/de/bla/richmodel/ui/binding/rcp/jface/formsupport/formpage/RichModelFormPage.java:83:27: Mögliches "this"-Escape vor vollständiger Initialisierung der Unterklasse
[3433] [warn] initialize(editor)
[3433] [warn]                   ^
[3433] [warn] /home/lefou/work/bla/bla/framework/ui.binding.rcp/src/de/bla/richmodel/ui/binding/rcp/jface/userinteraction/JfaceListMultipleSelectionDialog.java:82:25: Mögliches "this"-Escape vor vollständiger Initialisierung der Unterklasse
[3433] [warn] setTitle("some title")
[3433] [warn]                       ^
[3433] [warn] /home/lefou/work/bla/bla/framework/ui.binding.rcp/src/org/vafada/swtcalendar/RepeatingButton.java:55:33: Mögliches "this"-Escape vor vollständiger Initialisierung der Unterklasse
[3433] [warn] addMouseListener(new MouseAdapter() {
[3433] [warn] ==================public void mouseDown(MouseEvent event) {
[3433] [warn] ==========================cancelRepeater();
[3433] [warn] 
[3433] [warn] ==========================if (event.button == 1) { // Left click
[3433] [warn] ============================ __.cobuttonPressed(event.stateMask, event.time);
[3433] [warn] 
[3433] [warn] ============================ __.corepeater = new Repeater(event.stateMask);
[3433] [warn] ============================ __.cogetDisplay().timerExec(initialRepeatDelay, repeater);
[3433] [warn] ==========================}
[3433] [warn] ==================}
[3433] [warn] 
[3433] [warn] ==================public void mouseUp(MouseEvent event) {
[3433] [warn] ==========================if (event.button == 1) { // Left click
[3433] [warn] ============================ __.cocancelRepeater();
[3433] [warn] ==========================}
[3433] [warn] ==================}
[3433] [warn] kernel.kot})
[3433] [warn]                                 ^
[3433] [info] done compiling
[3455/3685] framework.uiBindingRcp.test.compile
@lihaoyi
Copy link
Member

lihaoyi commented Dec 23, 2024

Probably a consequence of #4053, which tried to minimize screen clears to avoid jittering in the prompt during refreshes

It seems that on your example, spaces and tab characters do not clear any text they are printed over. Not sure if it's a shell thing or a terminal thing. What shell and terminal are you using? Does it reproduce on other terminals?

@lihaoyi
Copy link
Member

lihaoyi commented Dec 23, 2024

You could also revert that commit and see if thet fixes the issue, just to confirm its the culprit

@lefou
Copy link
Member Author

lefou commented Dec 23, 2024

> mill --version
Mill Build Tool version 0.12.4
Java version: 21.0.5, vendor: Eclipse Adoptium, runtime: /opt/openjdk-bin-21.0.5_p11
Default locale: de_DE, platform encoding: UTF-8
OS name: "Linux", version: 6.1.90-gentoo-x86_64, arch: amd64

> bash --version
GNU bash, Version 5.2.37(1)-release (x86_64-pc-linux-gnu)

> konsole --version
konsole 24.08.3

@lefou
Copy link
Member Author

lefou commented Dec 23, 2024

Same issue with zsh 5.9

@lefou
Copy link
Member Author

lefou commented Dec 23, 2024

Reverting #4053 doesn't seem to fix it. I still see = in some locations where I would expect spaces. (Edit: I first forget to stop the running server and saw the same issue. Looks like, we no longer detect that automatically.)

Reverting seems to fix the issue. (I took 0.12.4 tag and reverted PR #4053.)

@lihaoyi
Copy link
Member

lihaoyi commented Dec 23, 2024

one mitigation could be to write a clear-line-right before writing every tab character. That would still minimize the amount of clearing going on in the common case, but at least would ensure tabs get printed on a properly blank canvas

@lihaoyi
Copy link
Member

lihaoyi commented Dec 23, 2024

@lefou maybe you could try adding \t to the regex below that implements the idea above, and see if that resolves the problem in your terminal? If so, I can then verify that it doesn't regress the original flickering problem on windows CMD, and then we can merge it and release as part of 0.12.5

.replaceAll("(\r\n|\n)", AnsiNav.clearLine(0) + "$1")

@lefou
Copy link
Member Author

lefou commented Dec 23, 2024

This seems to fix the issue for me.

diff --git a/main/util/src/mill/util/PromptLogger.scala b/main/util/src/mill/util/PromptLogger.scala
index c55e9dc73f0..12cbd91c79d 100644
--- a/main/util/src/mill/util/PromptLogger.scala
+++ b/main/util/src/mill/util/PromptLogger.scala
@@ -300,7 +300,7 @@ private[mill] object PromptLogger {
         // https://stackoverflow.com/questions/71452837/how-to-reduce-flicker-in-terminal-re-drawing
         dest.write(
           new String(buf, 0, end)
-            .replaceAll("(\r\n|\n)", AnsiNav.clearLine(0) + "$1")
+            .replaceAll("(\r\n|\n|\t)", AnsiNav.clearLine(0) + "$1")
             .getBytes
         )
       }

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

Successfully merging a pull request may close this issue.

2 participants