microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
160.51k stars 28.1k forks source link

Misaligned editor cursor movement with Hangul Jamo characters #207794

Open ruby3141 opened 3 months ago

ruby3141 commented 3 months ago

Does this issue occur when all extensions are disabled?: Yes

[!note] (updated 2024-03-18) It turns out the majority of described problem below was not from VSCode's bug. It was VSCodeVim to blame. Sorry for misinformation. Korean with Hangul Jamo still has problem tho.

Steps to Reproduce:

ruby3141 commented 3 months ago

Sorry for huge misinformation. VSCodeVim was not properly disabled during testing. Major fault for this problem was on VSCodeVim.

Everything works as intended EXCEPT for "Korean with Hangul Jamo", as of my reproduction.

ruby3141 commented 3 months ago

Record of current behavior, with extensions properly disabled

You can see that the cursor clearly bumps on Hangul Jamo line.

alexdima commented 3 weeks ago

The width of the glyphs is 100% controlled by the font. Have you tried a font like Inconsolata which is known for making all glyphs equallly wide?

ruby3141 commented 3 weeks ago

The glyphs of fonts I used in that reproduction are "uniformly" wide. More specifically, every "full-width" glyphs like CJK has exactly double the width of "half-width" glyph. (It's monospaced by design, but not technically. CJK programming fonts are usually behave like that.)

And VSCode simply consider full-width characters as "double sized" while calculating vertical movement. You can test that behavior with fonts like Last Resort, which is special purpose fallback font to indicate what Unicode block the "tofu"ed character is in, and has "equally" sized square box glyphs across every possible Unicode code point.

image With the font, the example text looks like this. highlighted characters are in the same "VSCode vertical line". And as you can see, unlike Hangul Syllables(5th line), Hangul Jamo(6th line) seems to be considered as "half-width", by the fact that it's in the same "Displayed vertical line" with "half-width" characters like English, German and Thai.

alexdima commented 3 weeks ago

And VSCode simply consider full-width characters as "double sized" while calculating vertical movement

That is correct, that is the current implementation. With the current stack we have, we delegate font rendering in the editor to the browser, so we don't implement text layouting ourselves. To make true width based vertical movement we would need to basically render the line the cursor would land on and do an offset search to find out what would be the closest column to the current horizontal offset. The current implementation hard-codes a heuristic that wide characters are twice as wide as narrow ones. This is done at https://github.com/microsoft/vscode/blob/0a417782b1781d39803b8f1791cfb22f8835ddae/src/vs/editor/common/core/cursorColumns.ts#L37-L55 and at https://github.com/microsoft/vscode/blob/0a417782b1781d39803b8f1791cfb22f8835ddae/src/vs/editor/common/core/cursorColumns.ts#L27-L35

Because of this, the current implementation also does not work correctly for proportional fonts. I acknowledge this behavior but will mark this as a feature request because the code is working as it was built, allowing for quick vertical movement while sacrificing correctness for non-monospace fonts or mostly non-programming text content.

VSCodeTriageBot commented 3 weeks ago

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

ruby3141 commented 3 weeks ago

@alexdima I don't meant about "move cursor vertically according to actual glyph size of current font" thing. It's that the Hangul Jamo block characters are not considered full width despite it is.

alexdima commented 3 weeks ago

@ruby3141 This is defined here -- https://github.com/microsoft/vscode/blob/d8abddce40aee62b166cbdc279e5c60fd9ec4fbf/src/vs/base/common/strings.ts#L677-L721 . Would you be interested in improving that with a PR?