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

Terminal: add support for showing/hiding the cursor (DECSET/DECRST 25, DECTCEM) #3093

Closed
egmontkob opened this issue Oct 6, 2019 · 5 comments · Fixed by #4902
Closed

Terminal: add support for showing/hiding the cursor (DECSET/DECRST 25, DECTCEM) #3093

egmontkob opened this issue Oct 6, 2019 · 5 comments · Fixed by #4902
Assignees
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-1 A description (P1) 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

@egmontkob
Copy link

Environment

Windows build number: Win32NT 10.0.18362.0
Windows Terminal version (if applicable): 0.5.2762.0

Steps to reproduce

printf '\e[?25l'

Expected behavior

The cursor should become invisible.

Actual behavior

The cursor remains visible and blinking. It is often misplaced on subsequent operations, though.

E.g. press the Up arrow, it brings back the previous printf command to the command line. Press the Left arrow, the cursor doesn't move. Press Backspace, the letter l is removed (that is, the logical cursor did indeed move to the left in the previous step), the cursor now blinks at the right edge.

@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 Oct 6, 2019
@DHowett-MSFT
Copy link
Contributor

Hey, we actually just got ConPTY support for rendering the cursor's hidden state (#2829). Right now, if conhost thinks the cursor is hidden it's not liable to leave it in the right place... I think we have another bug in here somewhere about the cursor being misplaced when deleting characters (and that's definitely something we shouldn't do :P)

@DHowett-MSFT DHowett-MSFT added Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Oct 6, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 6, 2019
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.0 milestone Oct 7, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 7, 2019
@DHowett-MSFT
Copy link
Contributor

Tearing off the Triage tag: this is now the master Task for supporting cursor hiding in WT. 😄

(when the cursor is hidden by the application, conhost is a lot more lax in keeping its location in sync with the connected terminal)

@DHowett-MSFT DHowett-MSFT added Issue-Task It's a feature request, but it doesn't really need a major design. and removed Issue-Bug It either shouldn't be doing this or needs an investigation. labels Oct 7, 2019
@DHowett-MSFT DHowett-MSFT changed the title Invisible cursor is visible and misplaced Terminal: add support for showing/hiding the cursor (DECSET/DECRST 25) Oct 7, 2019
@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. and removed Issue-Task It's a feature request, but it doesn't really need a major design. labels Nov 18, 2019
@j4james
Copy link
Collaborator

j4james commented Jan 5, 2020

Note that the escape sequence used by the XtermEngine to hide the cursor is incorrect:

// If the cursor was previously visible, let's hide it for this frame,
// by prepending a cursor off.
if (_lastCursorIsVisible)
{
_buffer.insert(0, "\x1b[25l");
_lastCursorIsVisible = false;
}

The \x1b[25l should be \x1b[?25l.

@zadjii-msft
Copy link
Member

@j4james holy crap that's a good catch

zadjii-msft added a commit that referenced this issue Jan 6, 2020
…lement cursor hiding, so this isn't really helpful by itself
@zadjii-msft zadjii-msft added the Priority-1 A description (P1) label Jan 9, 2020
@cinnamon-msft cinnamon-msft added Help Wanted We encourage anyone to jump in on these. v1-Scrubbed labels Jan 23, 2020
@DHowett-MSFT DHowett-MSFT changed the title Terminal: add support for showing/hiding the cursor (DECSET/DECRST 25) Terminal: add support for showing/hiding the cursor (DECSET/DECRST 25, DECTCEM) Jan 27, 2020
@zadjii-msft
Copy link
Member

note to self: I'm waiting on #4859 to merge because that PR's got most of the implementation of TerminalDispatch::_PrivateModeParamsHelper (where these sequences would need to be handled), and I don't want to create a massive merge conflict

@zadjii-msft zadjii-msft self-assigned this Mar 12, 2020
@ghost ghost added the In-PR This issue has a related PR label Mar 13, 2020
@ghost ghost closed this as completed in #4902 Mar 13, 2020
@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 Mar 13, 2020
ghost pushed a commit that referenced this issue Mar 13, 2020
Adds support for setting the cursor visibility in Terminal. Visibility
is a property entirely independent from whether the cursor is "on" or
not. The cursor blinker _should_ change the "IsOn" property. It was
actually changing the "Visible" property, which was incorrect. This PR
additionally corrects the naming of the method used by the cursor
blinker, and makes it do the right thing.

I added a pair of tests, one taken straight from conhost. In
copy-pasting that test, I took it a step further and implemented
`^[[?12h`, `^[[?12l`, which enables/disables cursor blinking, for the
`TerminalCore`. THIS DOES NOT ADD SUPPORT FOR DISABLING BLINKING IN THE
APP. Conpty doesn't emit the blinking on/off sequences quite yet, but
when it _does_, the Terminal will be ready.

## References
* I'd bet this conflicts with #2892
* This isn't a solution for #1379
* There shockingly isn't an issue for cursor blink state via conpty...?

## PR Checklist
* [x] Closes #3093
* [x] Closes #3499
* [x] Closes #4644
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
ArthurSonzogni added a commit to ArthurSonzogni/FTXUI that referenced this issue Jul 3, 2021
I finally got access to a computer using the Microsoft's Windows OS.
That's the opportunity to find and mitigate all the problems
encountered. This patch:

1. Introduce an option and a C++ definition to enable fallback for
   Microsoft's terminal emulators. This allows me to see/test the
   Microsoft output from Linux. This also allows Windows users to remove
   the fallback and target non Microsoft terminals if needed.

2. Microsoft's terminals do not provide a way to query if they support
   colors. As a fallback, assume true colors is supported.
   See issue:
   - microsoft/terminal#1040
   This mitigates:
   - #135

3. Microsoft terminal do not handle properly hidding the cursor. Instead
   the character under the cursor is hidden, which is a big problem. As
   a result, we don't enable setting the cursor to the best position for
   [input method editors](https://en.wikipedia.org/wiki/Input_method),
   It will be displayed at the bottom right corner.
   See:
   - microsoft/terminal#1203
   - microsoft/terminal#3093

4. The "cmd" on Windows do not properly report its dimension. Powershell
   works correctly. As a fallback, use a 80x80 size instead of 0x0.

5. There are several dom elements and component displayed incorrectly,
   because the font used is missing several unicode glyph. Use
   alternatives or less detailled one as a fallback.
ArthurSonzogni added a commit to ArthurSonzogni/FTXUI that referenced this issue Jul 4, 2021
I finally got access to a computer using the Microsoft's Windows OS.
That's the opportunity to find and mitigate all the problems
encountered. This patch:

1. Introduce an option and a C++ definition to enable fallback for
   Microsoft's terminal emulators. This allows me to see/test the
   Microsoft output from Linux. This also allows Windows users to remove
   the fallback and target non Microsoft terminals on Windows if needed.

2. Microsoft's terminal suffer from a race condition bug when reporting
   the cursor position:
   #136
   The mitigation is not to ask for the cursor position in fullscreen
   mode where it isn't really needed and request it less often.
   This fixes: #136

3. Microsoft's terminal do not handle properly hidding the cursor. Instead
   the character under the cursor is hidden, which is a big problem. As
   a result, we don't enable setting the cursor to the best position for
   [input method editors](https://en.wikipedia.org/wiki/Input_method),
   It will be displayed at the bottom right corner.
   See:
   - microsoft/terminal#1203
   - microsoft/terminal#3093

4. Microsoft's terminals do not provide a way to query if they support
   colors. As a fallback, assume true colors is supported.
   See issue:
   - microsoft/terminal#1040
   This mitigates:
   - #135

5. The "cmd" on Windows do not properly report its dimension. Powershell
   works correctly. As a fallback, use a 80x80 size instead of 0x0.

6. There are several dom elements and component displayed incorrectly,
   because the font used is missing several unicode glyph. Use
   alternatives or less detailled one as a fallback.
ArthurSonzogni added a commit to ArthurSonzogni/FTXUI that referenced this issue Jul 4, 2021
I finally got access to a computer using the Microsoft's Windows OS.
That's the opportunity to find and mitigate all the problems
encountered. This patch:

1. Introduce an option and a C++ definition to enable fallback for
   Microsoft's terminal emulators. This allows me to see/test the
   Microsoft output from Linux. This also allows Windows users to remove
   the fallback and target non Microsoft terminals on Windows if needed.

2. Microsoft's terminal suffer from a race condition bug when reporting
   the cursor position:
   microsoft/terminal#7583.
   The mitigation is not to ask for the cursor position in fullscreen
   mode where it isn't really needed and request it less often.
   This fixes: #136

3. Microsoft's terminal do not handle properly hidding the cursor. Instead
   the character under the cursor is hidden, which is a big problem. As
   a result, we don't enable setting the cursor to the best position for
   [input method editors](https://en.wikipedia.org/wiki/Input_method),
   It will be displayed at the bottom right corner.
   See:
   - microsoft/terminal#1203
   - microsoft/terminal#3093

4. Microsoft's terminals do not provide a way to query if they support
   colors. As a fallback, assume true colors is supported.
   See issue:
   - microsoft/terminal#1040
   This mitigates:
   - #135

5. The "cmd" on Windows do not properly report its dimension. Powershell
   works correctly. As a fallback, use a 80x80 size instead of 0x0.

6. There are several dom elements and component displayed incorrectly,
   because the font used is missing several unicode glyph. Use
   alternatives or less detailled one as a fallback.
ArthurSonzogni added a commit to ArthurSonzogni/FTXUI that referenced this issue Jul 4, 2021
I finally got access to a computer using the Microsoft's Windows OS.
That's the opportunity to find and mitigate all the problems
encountered. This patch:

1. Introduce an option and a C++ definition to enable fallback for
   Microsoft's terminal emulators. This allows me to see/test the
   Microsoft output from Linux. This also allows Windows users to remove
   the fallback and target non Microsoft terminals on Windows if needed.

2. Microsoft's terminal suffer from a race condition bug when reporting
   the cursor position:
   microsoft/terminal#7583.
   The mitigation is not to ask for the cursor position in fullscreen
   mode where it isn't really needed and request it less often.
   This fixes: #136

3. Microsoft's terminal do not handle properly hidding the cursor. Instead
   the character under the cursor is hidden, which is a big problem. As
   a result, we don't enable setting the cursor to the best position for
   [input method editors](https://en.wikipedia.org/wiki/Input_method),
   It will be displayed at the bottom right corner.
   See:
   - microsoft/terminal#1203
   - microsoft/terminal#3093

4. Microsoft's terminals do not provide a way to query if they support
   colors. As a fallback, assume true colors is supported.
   See issue:
   - microsoft/terminal#1040
   This mitigates:
   - #135

5. The "cmd" on Windows do not properly report its dimension. Powershell
   works correctly. As a fallback, use a 80x80 size instead of 0x0.

6. There are several dom elements and component displayed incorrectly,
   because the font used is missing several unicode glyph. Use
   alternatives or less detailled one as a fallback.
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-1 A description (P1) 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.

5 participants