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

Merged
merged 121 commits into from
Sep 26, 2024
Merged

Multi-line status prompt ticker #3577

merged 121 commits into from
Sep 26, 2024

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-24.at.3.01.58.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.

User-facing notes:

  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. 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.

  5. Console dimensions are propagated from the mill client to the mill server via a terminfo file specific to each server. This is used to size the prompt so it takes up the whole width and about 1/3 of the height. This happens every 100ms, so there may be some delay, but at least it means the prompt will right-size itself on next render. The regular cadence also means it shouldn't be a performance bottleneck

  6. For non-interactive runs which lack a terminal, prompt WxH defaults to 120x50, and we tweak the rendering: we no longer render blank lines to preserve prompt height stability, we render less frequently and only if the statuses change, and we add a footer so the bottom of the prompt is clearly marked.

  7. --ticker now defaults to true even for non-interactive runs, since the periodic display of prompt (I set to 60s intervals, if prompt changes) is useful e.g. for debugging stuck background jobs or CI runs.

Implementation Notes

  1. The logger needs to extend AutoCloseable, since it uses a background thread to keep the prompt UI up to date. This renders every 100ms (arbitrary) to try and balance between prompt readability and latency. We have additional delays built in to hiding a status line and then finally removing it, to try and preserve the height so it doesn't bounce up and down too much as the set of running tasks changes

  2. We re-use the ProxyStream classes to combine the stderr/stdout before re-splitting them. This allows us to perform some buffering, while simultaneously maintaining ordering of writes to each stream, while also allowing us to detect quiescence so we can only bother writing out a prompt when everything else is done and it won't get immediately over-written. ProxyStream.Pumper needed some tweaks to add hooks and remove client-specific assumptions

  3. 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
@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 25, 2024

One remaining issue here is that ad-hoc usages of .ticker in build code now misbehaves, e.g. the coursier Downloading indicators below which do not disappear because they happen on separate threads (and are thus counted as distinct from the original task) and do not have a corresponding .endTicker call:
Screenshot 2024-09-25 at 8 44 58 PM

Probably need to tweak the API and semantics of .ticker a bit to make this work properly, will take a look tomorrow

@lihaoyi lihaoyi merged commit 1780314 into com-lihaoyi:main Sep 26, 2024
24 checks passed
@lihaoyi lihaoyi mentioned this pull request Sep 26, 2024
lihaoyi added a commit that referenced this pull request Sep 26, 2024
This pulls in the new prompt from
#3577 so we can try it out
internally
@lefou lefou added this to the 0.12.0-RC3 milestone Sep 26, 2024
lihaoyi added a commit that referenced this pull request Oct 9, 2024
…3697)

This re-implements the ticker log prefixes that were left out in
#3577, both for meta-build
prefixes and nested-evaluation prefixes, e.g. the log prefix now looks
like `[build.mill-1]` `[mill-build/build.mill-1]` etc.

* We no longer need `dynamicTickerPrefix`, because we now perform this
context management by wrapping nested `PrefixLogger`s

* Required some gross mangling to consolidate `Logger` and `ColorLogger`
interfaces in order to make things properly stackable

* We needed to move the aggregation of keys to a top-down process,
rather than bottom up, since it is not easy to take an already prefixed
line `[1] my logs line` and append to the prefix `[1-2] my logs line`.
Thus we need to make sure the log line is properly prefixed at the
bottom-most logger, and the log line is passed up via
`unprefixedSystemStreams` to avoid redundant prefixing

<img width="976" alt="Screenshot 2024-10-09 at 7 36 05 PM"
src="https://github.com/user-attachments/assets/edc78141-5c09-4b5e-85ab-4cfda1603003">
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