-
Notifications
You must be signed in to change notification settings - Fork 2.8k
iTerm now supports OSC 9;4 (terminal window progress bar) #16506
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
Conversation
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
src/cargo/core/shell.rs
Outdated
| let iterm = std::env::var("TERM_PROGRAM").ok() == Some("iTerm.app".into()); | ||
|
|
||
| (windows_terminal || conemu || wezterm || ghostty) && stream.is_terminal() | ||
| (windows_terminal || conemu || wezterm || ghostty || iterm) && stream.is_terminal() |
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.
Do old versions gracefully ignore these escape codes?
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.
Good question! I'll follow up with @gnachman about this.
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.
Likely you can just do a quick check by forcing this on in older versions. The main reason this comes up is
- For some escape code sequences, there are bugs in older versions
- I think it was this escape sequence that partially overlapped with another one and terminals would do weird things if they didn't guard against that
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.
Older versions will treat them as a request to post a notification. Would it be useful to have a way to detect support for OSC 9 as statusbar?
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.
Depending on how its done, that could spam users. It would be good if we could do version detection to avoid those old versions.
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've updated iTerm2's feature reporting to support parameter packs. The documentation on the feature reporting mechanism is here:
https://iterm2.com/feature-reporting/
The website doesn't have the progress bar feature yet, but you can see the change to the docs here:
gnachman/iterm2-website@41b36d6
Here is a sample program for parsing and detecting the PROGRESS feature:
https://gist.github.com/gnachman/1aee2d5f440bc5b82e1db405ef67195f
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.
Oh, and my update will be in the next nightly build if you want to verify. It should be released in about 5 hours at https://iterm2.com/nightly/latest
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!
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.
|
Regarding CI, it was fixed in crate-ci/typos#1447, so we just need to update. I'll post a PR. |
This will unblock rust-lang#16506
### What does this PR try to resolve? This will unblock #16506 ### How to test and review this PR?
|
A rebase should resolve the CI failure. |
This comment has been minimized.
This comment has been minimized.
|
FWIW, it would be nice if we could avoid merge commits in pull requests. Feel free to rebase as needed. Keeping the history linear makes PRs easier to read and review, and ideally we’d only see a merge commit when the PR itself is merged, not merges from master along the way. |
Oops, my apologies 🙏 |
src/cargo/core/shell.rs
Outdated
| #[test] | ||
| fn term_features_progress_detection() { | ||
| // With PROGRESS feature ("P") | ||
| unsafe { std::env::set_var("TERM_FEATURES", "MBT2ScP") }; |
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.
Note that the default test harness (libtest) runs test in multi-threads. You might not want teset to affect each other. (though I know it is rare and doesn't matter right now).
What about we do this
fn term_features_has_progress(value: &str) -> boolSo that you can
let iterm = std::env::var(…).ok().map(term_features_has_progress).unwrap_or(false);
src/cargo/core/shell.rs
Outdated
| let ghostty = std::env::var("TERM_PROGRAM").ok() == Some("ghostty".into()); | ||
| // iTerm added OSC 9;4 support in v3.6.6, which we can check for. | ||
| // See https://github.com/rust-lang/cargo/pull/16506#discussion_r2706584034 for reference. | ||
| let iterm = term_features_has_progress(); |
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.
Should we check TERM_PROGRAM == "iTerm.app" first? While TERM_FEATURES looks like an iTerm stuff, it is too general that I am afraid some other emulator might have the same env with different intepretation.
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.
Yep this is a great callout, I'll add this momentarily.
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'd ask that you not add the TERM_PROGRAM check. It is part of an effort of the now-defunct terminals working group to develop a flexible capabilities reporting mechanism, which has been a sore spot for decades. At the moment only iTerm2 supports this so you're completely safe. It is very helpful to have popular apps use TERM_FEATURES to avoid fragmentation in the future. The trouble with using TERM_PROGRAM and related signals is that new terminals are forced to emulate existing ones for things to work right, and we ended up with a User-Agent-style mess many times over. This is our best chance to break out of that trap. Thank you for listening to my rant :)
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 view this as an allowlist and we can add more. Individual users can overwrite the allowlist using their config. I don't think terminals should lie about who they are.
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.
An allowlist effectively makes TERM_FEATURES iTerm-specific forever: other terminals won’t adopt a signal that major apps ignore by default, and users won’t extend an allowlist. This is a rare chance for terminals and applications to converge on a real capability-reporting mechanism instead of identity checks. Treating TERM_FEATURES as a pure capability signal helps avoid further fragmentation and reduces the long-term need for terminal-specific special cases.
I'm grateful for your help and I'll back off now. I'm glad OSC 9;4 is getting more use, anyway, and it'll be a nice addition to cargo.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Rust 1.93 just got released today, and we need to update things accordingly (see #16543), so yeah you need to rebase again. Sorry about the inconvenience. (The |
- Check for TERM_PROGRAM=iTerm.app before checking TERM_FEATURES - Safer env var testing per multi-threaded testing
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
All good, I just rebased again. |
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 working with us!
Update cargo submodule 14 commits in 85eff7c80277b57f78b11e28d14154ab12fcf643..efcd9f58636c1990393d495159045d9c35e43b8f 2026-01-15 16:18:08 +0000 to 2026-01-23 13:50:59 +0000 - chore(deps): update cargo-semver-checks to v0.46.0 (rust-lang/cargo#16548) - Increase cache_lock test timeout (rust-lang/cargo#16545) - iTerm now supports OSC 9;4 (terminal window progress bar) (rust-lang/cargo#16506) - chore: Updated compiler errors for Rust 1.93 (rust-lang/cargo#16543) - test(build-std): adjust snapshot (rust-lang/cargo#16539) - chore: bump to 0.96.0 (rust-lang/cargo#16538) - fix: update `resolve_all_features()` to filter pkg deps (rust-lang/cargo#16221) - fix: show implicit_minimum_version_req emitted source once per package (rust-lang/cargo#16535) - fix: `--remap-path-scope` stabilized in 1.95-nightly (rust-lang/cargo#16536) - feat(lints): Add non_kebab_case_bin lint (rust-lang/cargo#16524) - fix(rm): Suggest table flags when none are specified (rust-lang/cargo#16533) - fix(patch): clean up patch-related error messages (rust-lang/cargo#16498) - Store artifact deps in build unit dir (rust-lang/cargo#16519) - refactor(timings): reuse timing metric collection logic between `--timings` and `-Zbuild-analysis` (rust-lang/cargo#16497)
Update cargo submodule 14 commits in 85eff7c80277b57f78b11e28d14154ab12fcf643..efcd9f58636c1990393d495159045d9c35e43b8f 2026-01-15 16:18:08 +0000 to 2026-01-23 13:50:59 +0000 - chore(deps): update cargo-semver-checks to v0.46.0 (rust-lang/cargo#16548) - Increase cache_lock test timeout (rust-lang/cargo#16545) - iTerm now supports OSC 9;4 (terminal window progress bar) (rust-lang/cargo#16506) - chore: Updated compiler errors for Rust 1.93 (rust-lang/cargo#16543) - test(build-std): adjust snapshot (rust-lang/cargo#16539) - chore: bump to 0.96.0 (rust-lang/cargo#16538) - fix: update `resolve_all_features()` to filter pkg deps (rust-lang/cargo#16221) - fix: show implicit_minimum_version_req emitted source once per package (rust-lang/cargo#16535) - fix: `--remap-path-scope` stabilized in 1.95-nightly (rust-lang/cargo#16536) - feat(lints): Add non_kebab_case_bin lint (rust-lang/cargo#16524) - fix(rm): Suggest table flags when none are specified (rust-lang/cargo#16533) - fix(patch): clean up patch-related error messages (rust-lang/cargo#16498) - Store artifact deps in build unit dir (rust-lang/cargo#16519) - refactor(timings): reuse timing metric collection logic between `--timings` and `-Zbuild-analysis` (rust-lang/cargo#16497)
Update cargo submodule 14 commits in 85eff7c80277b57f78b11e28d14154ab12fcf643..efcd9f58636c1990393d495159045d9c35e43b8f 2026-01-15 16:18:08 +0000 to 2026-01-23 13:50:59 +0000 - chore(deps): update cargo-semver-checks to v0.46.0 (rust-lang/cargo#16548) - Increase cache_lock test timeout (rust-lang/cargo#16545) - iTerm now supports OSC 9;4 (terminal window progress bar) (rust-lang/cargo#16506) - chore: Updated compiler errors for Rust 1.93 (rust-lang/cargo#16543) - test(build-std): adjust snapshot (rust-lang/cargo#16539) - chore: bump to 0.96.0 (rust-lang/cargo#16538) - fix: update `resolve_all_features()` to filter pkg deps (rust-lang/cargo#16221) - fix: show implicit_minimum_version_req emitted source once per package (rust-lang/cargo#16535) - fix: `--remap-path-scope` stabilized in 1.95-nightly (rust-lang/cargo#16536) - feat(lints): Add non_kebab_case_bin lint (rust-lang/cargo#16524) - fix(rm): Suggest table flags when none are specified (rust-lang/cargo#16533) - fix(patch): clean up patch-related error messages (rust-lang/cargo#16498) - Store artifact deps in build unit dir (rust-lang/cargo#16519) - refactor(timings): reuse timing metric collection logic between `--timings` and `-Zbuild-analysis` (rust-lang/cargo#16497)
Update cargo submodule 14 commits in 85eff7c80277b57f78b11e28d14154ab12fcf643..efcd9f58636c1990393d495159045d9c35e43b8f 2026-01-15 16:18:08 +0000 to 2026-01-23 13:50:59 +0000 - chore(deps): update cargo-semver-checks to v0.46.0 (rust-lang/cargo#16548) - Increase cache_lock test timeout (rust-lang/cargo#16545) - iTerm now supports OSC 9;4 (terminal window progress bar) (rust-lang/cargo#16506) - chore: Updated compiler errors for Rust 1.93 (rust-lang/cargo#16543) - test(build-std): adjust snapshot (rust-lang/cargo#16539) - chore: bump to 0.96.0 (rust-lang/cargo#16538) - fix: update `resolve_all_features()` to filter pkg deps (rust-lang/cargo#16221) - fix: show implicit_minimum_version_req emitted source once per package (rust-lang/cargo#16535) - fix: `--remap-path-scope` stabilized in 1.95-nightly (rust-lang/cargo#16536) - feat(lints): Add non_kebab_case_bin lint (rust-lang/cargo#16524) - fix(rm): Suggest table flags when none are specified (rust-lang/cargo#16533) - fix(patch): clean up patch-related error messages (rust-lang/cargo#16498) - Store artifact deps in build unit dir (rust-lang/cargo#16519) - refactor(timings): reuse timing metric collection logic between `--timings` and `-Zbuild-analysis` (rust-lang/cargo#16497)
Update cargo submodule 14 commits in 85eff7c80277b57f78b11e28d14154ab12fcf643..efcd9f58636c1990393d495159045d9c35e43b8f 2026-01-15 16:18:08 +0000 to 2026-01-23 13:50:59 +0000 - chore(deps): update cargo-semver-checks to v0.46.0 (rust-lang/cargo#16548) - Increase cache_lock test timeout (rust-lang/cargo#16545) - iTerm now supports OSC 9;4 (terminal window progress bar) (rust-lang/cargo#16506) - chore: Updated compiler errors for Rust 1.93 (rust-lang/cargo#16543) - test(build-std): adjust snapshot (rust-lang/cargo#16539) - chore: bump to 0.96.0 (rust-lang/cargo#16538) - fix: update `resolve_all_features()` to filter pkg deps (rust-lang/cargo#16221) - fix: show implicit_minimum_version_req emitted source once per package (rust-lang/cargo#16535) - fix: `--remap-path-scope` stabilized in 1.95-nightly (rust-lang/cargo#16536) - feat(lints): Add non_kebab_case_bin lint (rust-lang/cargo#16524) - fix(rm): Suggest table flags when none are specified (rust-lang/cargo#16533) - fix(patch): clean up patch-related error messages (rust-lang/cargo#16498) - Store artifact deps in build unit dir (rust-lang/cargo#16519) - refactor(timings): reuse timing metric collection logic between `--timings` and `-Zbuild-analysis` (rust-lang/cargo#16497)
What does this PR try to resolve?
Per the iTerm version 3.6.6 release - "Add support for OSC 9;4 progress bars"
I added iTerm to
nextesthere and they suggested I add it to Cargo as well.How to test and review this PR?
I've test this locally, but it isn't really a unit test or integration test candidate.
Here's the output from my local iTerm console: