progress-addon by jerch · Pull Request #5251 · xtermjs/xterm.js

jerch · 2024-12-15T18:05:28Z

This addon implements ConEmu's progress sequence, for more context see #5250.

The addon tries to hide most of the sequence's shenanigans behind a convenient API:

const progressAddon = new ProgressAddon();
terminal.loadAddon(progressAddon);
progressAddon.register((state, value) => {
  // state: 0-4 integer
  // value: 0-100 integer
  
  // do your visualisation based on state/progress here
  ...
});

Fixes #5250.

Tyriar

Really solid 👏

Depending on app support, this could be really nice to integrate with the VS Code tab progress:

Tyriar · 2024-12-16T11:24:25Z

addons/addon-progress/typings/addon-progress.d.ts

+
+  /** progress object interface */
+  export interface IProgress {
+    state: 0 | 1 | 2 | 3 | 4;


Could pull this into type ProgressState?

Solved this by adding:

import type { ProgressState } from '../src/ProgressAddon';

It feels abit like a bad hack (cyclic import?), but works locally.

We shouldn't be referencing src from typing files. The more isolated the better as otherwise it can cause huge problems since libraries will suddenly be looking at and maybe linting against our source code, not just the types.

Hmm yeah, good point. Gonna try to find a better workaround. Sadly enums are tricky to use with d.ts files.

See my comment below.

addons/addon-progress/typings/addon-progress.d.ts

jerch · 2025-01-07T18:34:30Z

@Tyriar applied your remarks, up for review again.

jerch · 2025-01-07T20:25:39Z

@Tyriar Found a way to borrow the emitter ctor from xterm until #5283 is resolved. With this change things are back to normal with only 1.4kB minified 💪 :

asset addon-progress.js 1.4 KiB [compared for emit] [minimized] (name: main) 1 related asset
./out/ProgressAddon.js 2.25 KiB [built] [code generated]

Tyriar

Looks great, just some small polish things

addons/addon-progress/README.md

Tyriar · 2025-01-08T18:16:26Z

addons/addon-progress/src/ProgressAddon.ts

+    });
+    // FIXME: borrow emitter ctor from xterm, to be changed once #5283 is resolved
+    this._onChange = new (terminal as any)._core._onData.constructor();
+    this.onChange = this._onChange!.event;


Don' think the ! is needed?

Doesnt work without. My guess - the any cast above keeps the undefined as part of the type, so TS doesnt see this as undefined-guarded here.

All good, this is temporary

addons/addon-progress/typings/addon-progress.d.ts

Co-authored-by: Daniel Imms <[email protected]>

jerch · 2025-01-09T01:10:20Z

addons/addon-progress/typings/addon-progress.d.ts

+   * Progress state tracked by the addon.
+   */
+  export interface IProgressState {
+    state: 0 | 1 | 2 | 3 | 4;


@Tyriar Tried using a const enum inside the d.ts - this worked fine for TS itself and webpack build, but not with esbuild. So I changed it back to that integer union thingy. (Btw that plain union type is easier to understand for JS folks...)

That's fine 👌

Tyriar

👏

Will try adopt in vscode now microsoft/vscode#237564

Tyriar · 2025-01-09T13:09:51Z

FYI in case you didn't see this kovidgoyal/kitty#8011 (comment)

Tyriar · 2025-01-09T13:12:07Z

I see winget uses indeterminate progress, do you know of another application that implements value/% based? (cargo coming soon)

jerch · 2025-01-09T13:17:20Z

FYI in case you didn't see this kovidgoyal/kitty#8011 (comment)

Yeah, thats the reason why I had to exclude handling of OSC 9 that dont continue with ;4. There is a better more advanced spec for notifications than the old OSC 9 stuff, imho it got moved to OSC 99 with more advanced bells&whistles.

I see winget uses indeterminate progress, do you know of another application that implements value/% based? (cargo coming soon)

Nope, I dont. Systemd prolly uses that, but no clue in windows world.

Tyriar · 2025-01-09T13:38:14Z

winget's usage is a little unexpected since it seems to break up subtasks:

Not how I would have done it. I guess the progress state change should be debounced a little on my end to avoid a spinner flickering.

jerch · 2025-01-09T14:25:31Z

@Tyriar Yeah, debouncing might be needed. The sequence isnt all that good in its spec - it is also unclear, how long to show the progress indicator or when to remove it (I'd guess that it should be removed after some time of inactivity, or after a window focus toggle etc)

Tyriar · 2025-01-09T14:29:12Z

In my implementation I considered clearing the state after a command is run. I guess it's kind of a necessity if the proc crashes/ctrl+c doesn't handle it, etc.

jerch · 2025-01-09T14:36:37Z

I guess it needs a timeout running down, as we never know for sure when the terminal client app crashed or gave up for whatever reason.

Tyriar · 2025-01-09T14:39:57Z

I have it clearing when shell integration tells us any command finished: microsoft/vscode@86b257e

Seems like a fairly safe thing to do

jerch added 5 commits December 15, 2024 18:57

initial commit

6bb2210

fix esbuild script

80c5722

add to publish script

f0fd368

add to workflow bundle

569e854

fix handler example in readme

94ca05c

Tyriar requested changes Dec 16, 2024

View reviewed changes

Tyriar added this to the 6.0.0 milestone Dec 16, 2024

Tyriar self-assigned this Dec 16, 2024

jerch added 2 commits January 7, 2025 16:49

Merge branch 'master' into progress-addon

a1e85a4

import ProgressState type to d.ts

5badbd3

jerch mentioned this pull request Jan 7, 2025

Fix bloat introduced by vs/ module #5283

Closed

jerch added 3 commits January 7, 2025 19:21

use event from xterm.js instead of own impl

d7cebc3

fix integration test

828603c

make linter happy

c207edd

jerch added 2 commits January 7, 2025 20:36

fix docs

f516346

borrow emitter ctor from xterm

6a23276

Tyriar requested changes Jan 8, 2025

View reviewed changes

jerch and others added 3 commits January 9, 2025 01:06

Update addons/addon-progress/typings/addon-progress.d.ts

75e5cc1

Co-authored-by: Daniel Imms <[email protected]>

polish docs

96a3be3

rename types, remove rc import in d.ts

8101132

jerch commented Jan 9, 2025

View reviewed changes

Merge branch 'master' into progress-addon

6f6ce33

Tyriar mentioned this pull request Jan 9, 2025

Implement terminal sequence progress handler microsoft/vscode#237564

Closed

Add note in readme about full API

9caff8d

Tyriar approved these changes Jan 9, 2025

View reviewed changes

Tyriar enabled auto-merge January 9, 2025 12:58

Tyriar merged commit 330e91e into xtermjs:master Jan 9, 2025

Conversation

jerch commented Dec 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tyriar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jerch commented Jan 7, 2025

Uh oh!

jerch commented Jan 7, 2025

Uh oh!

Tyriar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerch Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jerch Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tyriar left a comment

Choose a reason for hiding this comment

Uh oh!

Tyriar commented Jan 9, 2025

Uh oh!

Tyriar commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jerch commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tyriar commented Jan 9, 2025

Uh oh!

jerch commented Jan 9, 2025

Uh oh!

Tyriar commented Jan 9, 2025

Uh oh!

jerch commented Jan 9, 2025

Uh oh!

Tyriar commented Jan 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jerch commented Dec 15, 2024 •

edited

Loading

jerch Jan 9, 2025 •

edited

Loading

jerch Jan 9, 2025 •

edited

Loading

Tyriar commented Jan 9, 2025 •

edited

Loading

jerch commented Jan 9, 2025 •

edited

Loading