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

Multi-line status prompt ticker #3577

Open
wants to merge 56 commits into
base: main
Choose a base branch
from

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Sep 18, 2024

fixes #3546

This PR introduces a new MultilinePromptLogger that allows the status of multiple in-progress threads to be reported and viewed during a Mill evaluation:

Screen.Recording.2024-09-19.at.2.29.11.PM.mov

MultilinePromptLogger is basically a minimal translation of the current ticker API to work with a multi-line prompt. There's probably a lot of other interesting improvements we can make now that we are multi-line, but this is a decent start that lets people know what their parallel build is doing. The UI works correctly both during scrolling and not-scrolling, and uses the same minimal ANSI codes that the existing ticker uses, so hopefully we won't hit any more edge cases that we aren't already hitting today

From an implementation perspective, MultilinePromptLogger is mostly a drop in replacement for the old PrintLogger, except:

  1. It prints multiple ticker lines at vertically at the bottom of the terminal, and has the logs printed above

  2. It requires that you use .endTicker() after every .ticker("...") call, so we can know where we should clear the ticker status (previously they always just got overriden by the next .ticker call)

  3. We need to introduce a withPaused{...} block so that when you're running REPLs and stuff the prompt is not shown

  4. The logger needs to extend AutoCloseable, since it uses a background thread to keep the prompt UI up to date

  5. We only can support logs which end with newlines. This means things like interactive progress bars and other ANSI magic won't work in the scrollback. This is a limitation of the current implementation that is hard to fix without going for more advanced techniques, but should be enough for the vast majority of logs and is a similar limitation as a lot of other tools.

  6. Ticker max width is hardcoded to <120 characters wide, with longer ticker strings being shortened in the middle. This width is arbitrary but should hopefully work for most projects, and we can figure out how to make it configurable or dynamic in a follow up

  7. I experimented with having the ticker logic live in the Mill client rather than server, which would make more sense, but we need the server to have the ability to enable/disable the ticker logic to run console and similar interactive tasks, and so it has to live in the server

The old ticker remains available at --disable-prompt. Further improvements can come after 0.12.0.

@lihaoyi lihaoyi changed the title [WIP] Multi-line status ticker Multi-line status prompt ticker Sep 19, 2024
@lefou
Copy link
Member

lefou commented Sep 19, 2024

I think with a lot of worker threads we should explore in the future if we can provide a multi-column output, where we cut line count by a factor at the price of shortened output.

@lefou
Copy link
Member

lefou commented Sep 19, 2024

Maybe we can fix the height for a given slot of time, e.g. 5 seconds. It can grow and shrink as needed, but should still feel stable enough due to the grace period.

@alexarchambault
Copy link
Contributor

alexarchambault commented Sep 19, 2024

Code such as this one allows to be kept updated when the terminal dimensions change, as the jline call to get them can be costly (so you don't want to call it on every update of the prompt)

@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 19, 2024

One issue I hit with org.jline.terminal.TerminalBuilder.terminal is that it only works in -i mode where we directly forward the client's stdin/out/err to the background process. In server mode, JLine is not able to properly determine terminal dimensions on the server. We may need to move that logic to the client and have some protocol for sending the dimensions to the server

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 this pull request may close these issues.

Improve Mill command line status reporting UI to display concurrent tasks
4 participants