microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.31k stars 8.28k forks source link

Feature Request: Taskbar progress indicator #3004

Closed dim5 closed 3 years ago

dim5 commented 5 years ago

Description of the new feature/enhancement

Add a taskbar progress indicator for displaying the status of the current long running process (when possible).

For example, this can be helpful to users unpacking files with 7z or running python scripts with tqdm.

ConEmu's documentation.

zadjii-msft commented 5 years ago

I could have sworn we had an internal issue for this feature, but it never got migrated to github it appears....

FOUND IT! MSFT:18258406. Here are the relevant notes:

sequence description
ESC ] 9 ; 4 ; st ; pr ST Set progress state on Windows 7 taskbar and ConEmu title. When st is 0: remove progress. When st is 1: set progress value to pr (number, 0-100). When st is 2: set error state in progress on Windows 7 taskbar

Also related documentation: ITaskbarList3::SetProgressValue

We should probably also make sure that we support the TBPF_INDETERMINATE state - for something like our own bcz.cmd, where we don't know how long the operation will take.

In conpty mode, we should just pass these sequences through. There's a good amount of precedent for such sequences in the code already.

EDIT: We should probably also have a couple settings for controlling this behavior:

showProgressInTab can probably be a follow up once https://github.com/microsoft/microsoft-ui-xaml/issues/1386 is complete

shmuelie commented 4 years ago

Would this make a good "starter" issue?

DHowett-MSFT commented 4 years ago

@SamuelEnglard

Perhaps! Anyone’s free to have a crack at any issue they find here unless the team’s actively assigned, and even then they just have to ask. It’s got a couple moving parts, but nothing impossible.

For anyone coming from Windows land: the reason the existing solution (calling GetConsoleWindow and using the Shell APIs directly to set progress on the window) doesn’t work is that GetConsoleWindow is an absolute abomination of an API and we never should have let a windowless application access the window it just happened to be hosted in. Circumventing the console and telling the shell to paint progress isn’t even something we can detect; we need applications to move to a standards-compliant or, at the least, quasi- standards-compliant, mechanism for setting progress.

zadjii-msft commented 4 years ago

More to the point, the work that I think is involved here is roughly:

This is kinda the bottom-up approach I'd take to implementing this myself for the Terminal. I'd probably want to start by looking at the last bullet point, actually. That way I'd know what shape the event that's going to get bubbled all the way through all these layers would need. I'd reckon that will line up pretty close with the VT sequence we'll be parsing at the start.

This would probably be a lot easier if you just wanted to implement it for conhost. Not nearly as much bubbling (but not no bubbling).

The events for setting the title will be really similar actually. There's a bit of extra logic for handling the title, depending on if the user has suppressApplicationTitle or any of those related settings, but you could probably ignore that for now.

j4james commented 3 years ago

We should probably also make sure that we support the TBPF_INDETERMINATE state - for something like our own bcz.cmd, where we don't know how long the operation will take.

Note that ConEmu does already support the indeterminate state, by setting the st parameter to 3. It's just not documented.

mintty commented 3 years ago

Why would the "green" and "red" options be supported but not the "yellow"? Since OSC sequence also support text parameters, this could be done in a compatible way like: ESC ] 9 ; 4 ; yellow ; pr ST

j4james commented 3 years ago

Why would the "green" and "red" options be supported but not the "yellow"?

The best place to ask this would be on on the ConEmu repo.

zadjii-msft commented 3 years ago

I totally forgot that there was also a yellow state to the taskbar as well. I think we should probably just make that st=4, for consistency with the other possible values for that parameter. That would map to the TBPF_PAUSED state

j4james commented 3 years ago

I think we should probably just make that st=4, for consistency with the other possible values for that parameter. That would map to the TBPF_PAUSED state

ConEmu already uses st 4 and 5 for something else (not exactly sure what), so it would need to be something outside that range. But if we want to extend this, I strongly recommend discussing it with the ConEmu devs first, since it's their sequence we're adopting. If they aren't interested, we can still choose to come up with the extension ourselves, but I think it's only polite to let them have the first crack at it.

zadjii-msft commented 3 years ago

Summoning @maximus5 - Could you help enlighten us what ESC ] 9 ; 4 ; 4 and ESC ] 9 ; 4 ; 5 do in ConEmu, and help weigh in on what sequence should be used for the TBPF_PAUSED state (maybe ESC ] 9 ; 4 ; 6 if 4 and 5 are already something else)

Maximus5 commented 3 years ago

Sure. Both ESC ] 9 ; 4 ; 4 ST and ESC ] 9 ; 4 ; 5 ST sequences are free for the moment. (There were some plans which were never implemented). The ESC ] 9 ; 4 ; 4 ST for TBPF_PAUSED looks good.

zadjii-msft commented 3 years ago

Thanks for the quick reply @Maximus5!

mixmastamyk commented 3 years ago

This unfortunately conflicts with iterm2 and kitty use of OSC 9. Not the end of the world since neither supports the others notification, but definitely problematic.

mintty commented 3 years ago

Both could coexist as long as messages do not begin with "4;"...

j4james commented 3 years ago

This unfortunately conflicts with iterm2 and kitty use of OSC 9.

This was discussed in the PR and the terminal-wg issue that you linked. We reached the conclusion that WT probably wouldn't need to support iterm2's use of OSC 9, because if we did decide to implement notifications, we'd be more likely to go with OSC 777, since that's more widely supported.

mixmastamyk commented 3 years ago

Ok, but what about from the app developer's point of view? I just implemented the progress reporting in my library, and it's nice, thanks.

However, when I run my python script on kitty (and presumably iterm and others?) I get a ton of bogus desktop notifications "4;0;10", "4;0;20", etc.... instead. I had to put this BS in my code to work around it:

if os.name = 'nt':
    # impl 1 ...        
else:
    # impl 2 ...

The least bad solution was probably using another number. As I read the discussion I realized Mr. Kovid is difficult, but I think he was right, we're not running out of integers any time soon.

It's a shame that iterm and conemu chose conflicting sequences ~ten years ago, and there's nothing we can do about that. But now that folks are at least in some communication there's no reason to double down on them. ;-)

j4james commented 3 years ago

As I read the discussion I realized Mr. Kovid is difficult, but I think he was right, we're not running out of integers any time soon.

If he really cared about the conflict, though, he didn't have to implement OSC 9 as a notification sequence - kitty already supports notification through another OSC sequence. And while I can sympathise with iTerm2, gnachman didn't seem particular attached to OSC 9 when the subject of notifications was being discussed, so it's quite possible he might drop it one day.

DHowett commented 3 years ago

"We're not running out of integers,"

"so I will continue using this one and you should change thank you."

mintty commented 3 years ago

mintty also implementes CSIN%q as an alternative sequence; see https://github.com/mintty/mintty/wiki/CtrlSeqs#progress-bar but note the different numbering

mixmastamyk commented 3 years ago

As an application developer, I'd like one sequence that works as many places as possible without conflicts…

zadjii-msft commented 3 years ago

If you want consistency, I don't think the interpretation of escape sequences is a good place to be 😆

There's just already so much fragmentation in this space, especially with OSC sequences. No one really standardized them in the first place, so everyone just went wild west claiming whatever they wanted. Not much we can do about it now unfortunately.

Curious to me that iTerm2 chose OSC 9. Looks like the sequence must be very old in iTerm2: https://github.com/kovidgoyal/kitty/issues/1474#issuecomment-475118290. Plus, ConEmu has been using OSC 9 as their prefix for years. So it really doesn't make sense to me to double down on OSC9 -> notification. It seems like it was an oversight when it was authored, the current author isn't attached to it, and there are better (non-conflicting alternatives).

We could also support the mintty version of the sequence if that doesn't conflict too bad. Don't see why not.

j4james commented 3 years ago

We could also support the mintty version of the sequence

If the point is to have a sequence that won't break kitty, then that probably isn't going to help. The last version I tested had issues with intermediates (at least some of them), which caused the sequences to be misinterpreted as something else. You may not get a notification popup, but you'll probably get some other weird breakage.

shmuelie commented 3 years ago

When in doubt, offer the option to change which OSC is used in settings?

DHowett commented 3 years ago

That only complicates the feature matrix, though. For @mixmastamyk's "application developer" case, now it's just unreliable rather than "consistently different from iTerm2."

shmuelie commented 3 years ago

That is true, would need a way to tell Terminal "this application uses OSC 9 for notifications, please do..."

zadjii-msft commented 3 years ago

Man if only there was some sort of database that an app could use to look up "this is how you emit progress bar updates, this is how you send notifications, this is how you set the colors" for various different terminals 😝