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.

Sign up for GitHub

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

Jump to bottom

Double cursors in neovim terminal buffer (and fzf, probably more) #3499

Closed
schffp opened this issue Nov 9, 2019 · 8 comments · Fixed by #4902
Closed

Double cursors in neovim terminal buffer (and fzf, probably more) #3499

schffp opened this issue Nov 9, 2019 · 8 comments · Fixed by #4902
Assignees
@zadjii-msft
Labels
Area-VT Virtual Terminal sequence support 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
Terminal v1.0

Comments

@schffp
Copy link

schffp commented Nov 9, 2019

Environment

Windows build number: 10.0.18362.418
NVIM v0.3.8

Steps to reproduce

Open Neovim in WSL Windows Terminal and open a terminal buffer (:ter). Then enter insert mode.

Actual behavior

There will be two cursors.
@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 Nov 9, 2019
@zadjii-msft zadjii-msft added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Nov 11, 2019
@zadjii-msft
Copy link
Member

@schffp Could you share screenshots of what you're seeing? What Windows Terminal version are you running?

@schffp
Copy link
Author

schffp commented Nov 12, 2019 •
edited
Loading

@zadjii-msft Sure! I'll attach screenshots of fzf.vim and the terminal buffer. I'm using version 0.6.2951.0 of Windows Terminal.

fzf
terminalbuffer

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Nov 12, 2019
@zadjii-msft
Copy link
Member

Okay I think I see what's happening here. wt vs conhost:
image
image

I bet that this is due to the Windows Terminal not currently supporting the "cursor off" VT sequence. This is tracked over in #3093, but I'm going to cautiously leave this open to make sure this case is also fixed when we implement that.

@zadjii-msft zadjii-msft added Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Nov 14, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Nov 14, 2019
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Nov 14, 2019
@mkarbo
Copy link

mkarbo commented Jan 13, 2020

I also have this issue, which makes it pretty much impossible for me to use the terminal application

@zadjii-msft zadjii-msft added the Priority-2 A description (P2) label Jan 22, 2020
@zadjii-msft zadjii-msft self-assigned this Mar 11, 2020
@zadjii-msft zadjii-msft mentioned this issue Mar 13, 2020
Merged
5 tasks
@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 removed the In-PR This issue has a related PR label Mar 13, 2020
ghost pushed a commit that referenced this issue Mar 13, 2020
@zadjii-msft
Add support for setting the cursor visibility in Terminal (#4902)
38058a7 
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
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Mar 13, 2020
@surajp
Copy link

surajp commented Mar 17, 2020

Has this fixed been deployed to the store yet? I'm still seeing the issue

@DHowett-MSFT
Copy link
Contributor

@surajp you'll know when we make a new release

like the one we sent out 6 minutes ago with an accompanying blog post

@DHowett-MSFT
Copy link
Contributor

The bot also usually comments on resolved issues, but it doesn't seem to be doing so right now.

@ericktucto
Copy link

I also have a double cursor, and I'm on linux (Ubuntu 20.04.3) using the xfce terminal

I have Nvim 0.5 (AppImagen)
gnome-shell-screenshot-5DRX90

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-VT Virtual Terminal sequence support 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
Milestone
Terminal v1.0
Development

Successfully merging a pull request may close this issue.

Add support for setting the cursor visibility in Terminal microsoft/terminal
7 participants
@surajp @DHowett-MSFT @zadjii-msft @ericktucto @schffp @mkarbo @cinnamon-msft