Skip to content
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

Incomplete support of ConEmu's OSC 9;4 sequences #9960

Closed
chausner opened this issue Apr 26, 2021 · 4 comments · Fixed by #10024
Closed

Incomplete support of ConEmu's OSC 9;4 sequences #9960

chausner opened this issue Apr 26, 2021 · 4 comments · Fixed by #10024
Labels
Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@chausner
Copy link
Contributor

chausner commented Apr 26, 2021

#8055 implemented the ConEmu-specific VT sequence for displaying progress bars in the Windows task bar. According to https://conemu.github.io/en/AnsiEscapeCodes.html#ConEmu_specific_OSC, the progress value number in the sequence is optional when st is set to 2 or 4 but Windows Terminal currently does not recognize these sequences. For example:

> python
print("\x1b]9;4;2;\x1b\\")

switches the progress state to "Error" in ConEmu but does not have any effect in Windows Terminal.

Even though it is not explicitly written in the documentation linked above, ConEmu also allows leaving out the progress value when pr is set to values other than 2 and 4. This makes sense since for clearing the progress with pr = 0 or switching to indeterminate state pr = 3, the progress value does not have any meaning anyway.

I suggest to adjust the implementation in Windows Terminal to properly recognize sequences where the pr value has been left out in order to be compatible with ConEmu. The expected behaviour in this case would be to just switch the progress state but leave the progress value at the previously set value.

/cc @DHowett

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 26, 2021
@skyline75489 skyline75489 added Area-VT Virtual Terminal sequence support Product-Terminal The new Windows Terminal. labels Apr 27, 2021
@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) labels Apr 27, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 27, 2021
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Apr 27, 2021
@j4james
Copy link
Collaborator

j4james commented Apr 27, 2021

Wow. This is way more complicated than I originally thought.

Default and missing parameter values map to 0 (I believe we already do this). But when the progress value is 0, that has special meaning for states 2 and 4. As far as I can make out, the rules are as follow:

  • If the state is 0, the current progress value should be set to 0, regardless of the given value.
  • If the state is 3, the current progress value should be left unchanged, regardless of the given value.
  • If the state is 1, the current progress value is updated with the given value, clamped 0 to 100.
  • If the state is 2 or 4, the current progress value is only updated with the given value if greater than 0. If 0, it inherits the existing value. If the existing value is 0, then it appears to be set to the minimum non-zero value.

Some examples:

  • 9;4;1;50 then 9;4;2 sets the progress value to 50
  • 9;4;0;50 then 9;4;2 sets the progress value to something like 10 (the minimum) not 50
  • 9;4;1;50 then 9;4;3;75 then 9;4;2 sets the progress value to 50 not 75

@j4james
Copy link
Collaborator

j4james commented Apr 27, 2021

Default and missing parameter values map to 0 (I believe we already do this).

Actually it looks like I'm wrong about this too. The OSC commands don't use the standard parameter parser, so this doesn't handle the cases where the semicolon is present but the parameter value isn't, e.g. 9;4;1 will be correctly interpreted as 9;4;1;0, but 9;4;1; (with a trailing semicolon) will fail.

@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Apr 29, 2021
@zadjii-msft
Copy link
Member

@PankajBhojwani as an fyi

@ghost ghost added the In-PR This issue has a related PR label May 3, 2021
@ghost ghost closed this as completed in #10024 May 5, 2021
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels May 5, 2021
ghost pushed a commit that referenced this issue May 5, 2021
## Summary of the Pull Request
When we parse OSC 9;4, allow a trailing semicolon (i.e. allow `9;4;` or something like `9;4;3;`). 

## PR Checklist
* [x] Closes #9960 
* [X] Tests added/passed

## Validation Steps Performed
OSC 9;4 sequences with or without trailing semicolons work
DHowett pushed a commit that referenced this issue May 24, 2021
## Summary of the Pull Request
When we parse OSC 9;4, allow a trailing semicolon (i.e. allow `9;4;` or something like `9;4;3;`).

## PR Checklist
* [x] Closes #9960
* [X] Tests added/passed

## Validation Steps Performed
OSC 9;4 sequences with or without trailing semicolons work

(cherry picked from commit f518235)
@ghost
Copy link

ghost commented May 25, 2021

🎉This issue was addressed in #10024, which has now been successfully released as Windows Terminal Preview v1.9.1445.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants