microsoft
Address misplaced cursor
Addresses #156788.
It looks like the "centering" is the main culprit causing my issue.
Test this by the following steps:
- configure a wide-ish line cursor (so the letters underneath show up inside the cursor)
- observe how with this PR the letter is no longer shifted by a pixel inside the cursor when not in text column 0
This is most visible on high-DPI screens where a pixel translates to several real pixels.
If we want to keep the "centering", or if we actually want to truly center the line cursor by half its width, we'd need to add left padding to the cursor element so that the shift left is balanced by the content shifting right. It would mean adding left padding (or all paddings) to fastDomNode. Let me know if you'd like that.
@alexdima I've realized that my commit message might be seen as criticising your earlier commit, sorry if it came across that way. I hadn't realized that this tweak was part of a fix for an earlier issue.
It doesn't seem that issue #55640 recurs with my reversal of that "centering"; but if you'd prefer, I can look at the approach outlined above that can shift the cursor without moving the text inside it.
I understand; I've updated the PR so the cursor does shift by 1px but it adds left padding so the text is where it's supposed to be. What do you think @alexdima ?
I've spotted something else: it looks like the last line of real code which returns width: 2 may want to return the actual width of the cursor. But it may be harmless.
I've updated the PR so the cursor does shift by 1px but it adds left padding so the text is where it's supposed to be.
The change looks alright in principle, but I think the extra padding is making the default cursor "fatter". Here it is running from source on macOS with default settings:
| Before the proposed change | With the proposed change |
|---|---|
 |
 |
I've spotted something else: it looks like the last line of real code which returns width: 2 may want to return the actual width of the cursor. But it may be harmless.
That's a good catch! I've created https://github.com/microsoft/vscode/issues/164484 to track fixing that.
