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

Incorrect cursor position on full lines #15888

Closed
zamadatix opened this issue Aug 28, 2023 · 8 comments · Fixed by #17510
Closed

Incorrect cursor position on full lines #15888

zamadatix opened this issue Aug 28, 2023 · 8 comments · Fixed by #17510
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty
Milestone

Comments

@zamadatix
Copy link

Windows Terminal version

1.7.11461.0

Windows build number

10.0.22621.0

Other Software

No response

Steps to reproduce

Create a character string which is one character short of the full line width:

image

Add the final character and note the cursor appears one character left of where it should:

image

Expected Behavior

Cursor should appear at the end of the line when the line is full.

Actual Behavior

Cursor appears one character before the end of the line. Operationally the cursor functions according to the correct position ("delete" does nothing and "backspace" removes the "L"). This is an off-by-one error on just the display side.

@zamadatix zamadatix added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 28, 2023
@DHowett
Copy link
Member

DHowett commented Aug 29, 2023

Thanks for reporting this!

You'd be surprised -- this is actually working as intended! Applications are allowed to write text all the way to the edge of the screen without incurring line wrapping. However, the cursor cannot go off the edge of the screen to indicate that the next character will go into outer space.

Different terminal emulators handle this differently:
image

It looks funny when the default cursor is set to the vertical bar... but the cursor technically occupies a cell, and not a place between cells.

/cc @j4james, as I am probably getting some details wrong 😄

@DHowett DHowett added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 29, 2023
@pere3
Copy link

pere3 commented Aug 29, 2023

Hi, i wonder if this is the same issue i'm having with vim on windows terminal, where cursor always appears one character to the left from the end of the line?
2023-08-29_20-13-04

@DHowett
Copy link
Member

DHowett commented Aug 29, 2023

In your case (@pere3), the position of the cursor is controlled by vim. 😄

@zamadatix
Copy link
Author

zamadatix commented Aug 29, 2023

Hey DHowett, thanks for taking the time to respond and spread some knowledge 😊:

You'd be surprised -- this is actually working as intended! Applications are allowed to write text all the way to the edge of the screen without incurring line wrapping. However, the cursor cannot go off the edge of the screen to indicate that the next character will go into outer space.

Different terminal emulators handle this differently: image

It looks funny when the default cursor is set to the vertical bar... but the cursor technically occupies a cell, and not a place between cells.

Hmm that's fair. I guess I was just used to the behavior of the original console where even the full sized cursor would wrap:

image

If I remember the console APIs correctly this is a behavior the app can choose to override but idk, to me it makes more sense for the cursor to always display where it is but maybe there is nothing saying that's how it's supposed to be.

There is another inconsistency I've noticed with the way the end of line is handled. If you type to the end of a line like this:

image

You can press the right arrow and nothing happens, as would be expected given the current positioning logic. If you instead press left then right (i.e., leave the position and return back to it) the cursor follows the "move to next line" behavior even though the position is the same as before when it followed the "stay at end of line" behavior:

image

I'm not sure if this behavior is inherent to the way the line wrapping logic is supposed to work in this method but it certainly feels inconsistent to someone unfamiliar.

@DHowett I'll leave it up to you to divine if this second behavior would be categorized as a bug or not. In either scenario, I'd like to make the case that it (feels) more consistent to just wrap the cursor to the next line as the default behavior, if both are valid, but agree that may not fit under the "bug" tag.

@microsoft-github-policy-service microsoft-github-policy-service bot 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 Aug 29, 2023
@j4james
Copy link
Collaborator

j4james commented Aug 29, 2023

You'd be surprised -- this is actually working as intended! Applications are allowed to write text all the way to the edge of the screen without incurring line wrapping. However, the cursor cannot go off the edge of the screen to indicate that the next character will go into outer space.

@DHowett I think there's more to this, though, because it works differently in conhost. I had a brief look at what was going on and it appears that pwsh is using a mix of VT and legacy APIs. When you type a character at the end of the line, it redraws the line in VT mode, leaving the cursor in the last column. But it also then calls SetConsoleCursorPosition to manually reposition the cursor on the next line. That final cursor position doesn't seem to be forwarded over conpty. So I think there is likely a genuine bug here.

@DHowett
Copy link
Member

DHowett commented Aug 29, 2023

Huh! I get to be the one who is surprised! Thanks for looking, James. 😄

@lhecker
Copy link
Member

lhecker commented Aug 30, 2023

I was looking through other issues and couldn't find an answer to that, so...
If we ever move the rendering of the viewport margins into the renderers, could we perhaps draw the cursor inside the margin (if the margin is >0)? That would be different to other terminals, but would pretty cool.

BTW while writing this I just noticed that our margins aren't quite uniform...
Edit: Opened #15903

@lhecker lhecker removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Attention The core contributors need to come back around and look at this ASAP. labels Aug 30, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Aug 30, 2023
@lhecker lhecker added Product-Conpty For console issues specifically related to conpty Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Priority-2 A description (P2) and removed Needs-Tag-Fix Doesn't match tag requirements labels Aug 30, 2023
@lhecker lhecker added this to the Terminal v1.20 milestone Aug 30, 2023
@j4james
Copy link
Collaborator

j4james commented Aug 31, 2023

could we perhaps draw the cursor inside the margin (if the margin is >0)?

If we do, it should definitely be an option, preferably off by default, because from a standards point of view that's just wrong. When the terminal is in this state, the cursor position is explicitly defined as the last column. The reported position is going to be the last column. Any movement operation is going to be relative to the last column. If you're rendering the cursor somewhere else, it's going to appear broken. Maybe it's a personal preference, but if I saw a terminal doing this I'd assume it was a bug.

@zadjii-msft zadjii-msft added the Help Wanted We encourage anyone to jump in on these. label Jan 31, 2024
@zadjii-msft zadjii-msft modified the milestones: Terminal v1.20, Backlog Jan 31, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Aug 1, 2024
@lhecker lhecker closed this as completed in 450eec4 Aug 1, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants