progress-addon

[jerch]

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]

Could pull this into type ProgressState?

[jerch]

Solved this by adding:

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

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

[Tyriar]

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.

[jerch]

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

[jerch]

See my comment below.

[jerch]

@Tyriar applied your remarks, up for review again.

[jerch]

@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

[Tyriar]

Don' think the ! is needed?

[jerch]

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.

[Tyriar]

All good, this is temporary

[jerch]

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

[Tyriar]

That's fine 👌

[Tyriar]

👏

Will try adopt in vscode now microsoft/vscode#237564

[Tyriar]

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

[Tyriar]

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

[jerch]

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]

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]

@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]

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]

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]

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

Seems like a fairly safe thing to do