-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Emit progress state escape codes #11436
Emit progress state escape codes #11436
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) soon. Please see the contribution instructions for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pull request, and sorry for the long wait.
This is a UI change. Could you demonstrate demo GIFs or videos for it? It would be helpful if you could provide the result on both msvc and mingw.
In addition, do you have the official documentation at hand about this escape code? CodeEmu doc doesn't seem to be official I feel like.
@@ -39,6 +40,25 @@ struct Format { | |||
max_print: usize, | |||
} | |||
|
|||
enum ProgressCode { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is Windows-only feature, right? Is it possible to add #[cfg(windows)]
for all relevant code and items?
Also, having doc comments on each new items would be fantastic. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think that other terminal emulators may also implement those, but I can't verify now.
It seems like per alacritty/alacritty#5201, it won't get this support, however.
As per microsoft/terminal#8055, the ConEmu style is the implementation reference. |
Emit progress state escape codes when drawing progress bar.
888a723
to
b60dc0d
Compare
Here is a screen recording of running 2022-12-21.14-57-40.mp4Notice the bar in the taskbar, and the circle getting bigger / fuller at the top left corner. |
Thanks for the screen recording! Unfortunately on macOS it doesn't seem to compile: https://github.com/rust-lang/cargo/actions/runs/3749845063/jobs/6368801382 I am still uncertain the adoption rate of this non-standard escape code. Does the majority of terminal emulators handle or ignore this well? Sorry being so conservative but I've seen some issue around “escape code not recognized” sometimes. That's also why I suggested adding |
Current testing progress:
Procedure:
|
#11436 (comment) was a good try. Thank you. I'll probably split Support to two columns:
Marked this as |
Personally I love this kind of good UI enhancement with the system. We just need more data to make sure it is safe for most people. Going to close due to inactivity. We still look for people investing in surveying. Thank you anyway! Edited: track in #11432. |
### What does this PR try to resolve? A few terminal emulators support progress output to Windows taskbar. `winget` uses this to show install progress. Notably, Windows Terminal [recently (2020) added support](microsoft/terminal#8055) for ANSI codes [specified](https://conemu.github.io/en/AnsiEscapeCodes.html#ConEmu_specific_OSC) in ConEmu (another terminal emulator for Windows) documentation. Also, in "[Learn Windows](https://learn.microsoft.com/en-us/windows/terminal/tutorials/progress-bar-sequences)". I've found the previous attempt to add this feature: #11436 As per @weihanglo's request, I've added the config option to enable/disable this feature. **It's enabled on supported terminal emulators.** Fixes #11432 FCP: #14615 (comment) ### How should we test and review this PR? Run `cargo build` in Windows Terminal with configuration option `term.progress.taskbar` set to `true`. ### Not sure - [x] Should all the code be `#[cfg(windows)]`? Probably no, because the feature is also usable in WSL. > Solved by introducing heuristic based on environment variable set by terminal - [ ] If Ctrl+C is pressed, a progressbar will stay in a last state forever (shown in the ConEmu video). `winget` is also behaves like alike. I've experimented with `ctrl_c handler` and it's totally fixable. - [x] `Enabled` is a sensible default for WSL because it works on linux builds in Windows Terminal too > Solved by introducing heuristic based on environment variable set by terminal - [x] Downloading stage may produce unpleasant blinking due to a rapid 0-100 changes > Solved by not displaying bar when downloading and using indeterminate state in other cases so amination don't reset ### TLDR * An `term.progress.taskbar` option with bool type is added * On Windows Terminal and ConEmu is enabled by default * If enabled reports build progress to taskbar icon and/or tab header ### Videos https://github.com/user-attachments/assets/48bb648a-e819-490e-b3ac-3502bc5f2f3a https://github.com/user-attachments/assets/1d7ddf7a-34dd-4db1-b654-e64d7170798e
Emit progress state escape codes when drawing progress bar.
Fixes #11432