Skip to content

Add ambiguouswidth option - #4206

Open
aron-intframe wants to merge 2 commits into
micro-editor:masterfrom
aron-intframe:ambiguous-width
Open

Add ambiguouswidth option#4206
aron-intframe wants to merge 2 commits into
micro-editor:masterfrom
aron-intframe:ambiguous-width

Conversation

@aron-intframe

Copy link
Copy Markdown

Description

Characters with the East Asian Ambiguous width property (Greek, Cyrillic, accented Latin) are measured as two columns whenever LANG/LC_* names a CJK locale, as go-runewidth autodetects. Most terminals draw them one column wide, so text is misaligned and the cursor sits in the wrong column (#1124). This adds an ambiguouswidth option, shaped after truecolor.

Measured in a pty, LANG=ja_JP.UTF-8, buffer let αβγ = 0: with auto tcell moves two columns per rune (α ESC[1;9H β ESC[1;11H γ ESC[1;13H, cursor ends at column 17); with single the runes are contiguous and the cursor ends at column 14. glibc 2.39 and musl 1.2.5 wcwidth() both return 1 for these code points under ja_JP.UTF-8, so the doubling is micro's choice, not libc's.

Open question: the default. I kept auto, so nothing changes; single would match wcwidth and the editors mentioned in the issue.

User-Facing Changes

New global-only option ambiguouswidth (auto/single/double), applied on set and documented in options.md. Nothing changes at the default.

Tests

TestSetAmbiguousWidth in internal/util; go test ./... passes. Mutation-checked: making single set wide, or dropping the DefaultCondition sync, each turns it red. Also reconciled against the UAX #11 15.1.0 file (what go-runewidth v0.0.16 is generated from): of its 138,739 Ambiguous code points, 138,626 flip 1->2 and 113 are zero-width combining marks; 0 of 209,089 non-Ambiguous ones change.

The width of characters with the East Asian Ambiguous width property is
currently decided by the locale environment variables, since that is what
go-runewidth autodetects. Terminals usually draw these characters one
column wide regardless of the locale, so in an East Asian locale micro
misaligns the text and puts the cursor in the wrong column, with no way
to override it.

Add a global-only `ambiguouswidth` option, in the same shape as
`truecolor`: `auto` (the default, i.e. the current behavior), `single`
and `double`.

@Andriamanitra Andriamanitra left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR seems like AI slop but this part does actually seem to fix the problem:

runewidth.DefaultCondition.EastAsianWidth = runewidth.EastAsianWidth

However I'm not sure what the use case for the ambiguouswidth setting is, couldn't we simply set runewidth.DefaultCondition.EastAsianWidth in the init function in util.go?

Or, even better, just update our go-runewidth dependency. The latest version does something equivalent automatically. (EDIT: nevermind, the old version also does it automatically so this PR is just nonsense – the dependency update that actually helped was tcell, not go-runewidth)

Writing runewidth.EastAsianWidth has no effect on measurement: go-runewidth
reads DefaultCondition, and syncs it from that package variable only once in
its own init(). Drop the redundant write and keep the assignment that matters.
@aron-intframe

Copy link
Copy Markdown
Author

Re-ran all of this on a fresh clone of master; you are right about the quoted line, and it is already gone from the branch.

1. runewidth.DefaultCondition.EastAsianWidth = runewidth.EastAsianWidth is a no-op. go-runewidth's own init() calls handleEnv(), which already syncs DefaultCondition. Probe built inside the micro module, LANG=ja_JP.UTF-8:

go-runewidth v0.0.16 (go.mod), LANG=ja_JP.UTF-8
runewidth.EastAsianWidth                  = true
runewidth.DefaultCondition.EastAsianWidth = true (after go-runewidth's own init)
runewidth.RuneWidth(U+03B1 alpha)         = 2
after the line quoted in the review       = true, RuneWidth(U+03B1) = 2

That line was in the first commit; 5543c6f (pushed, second commit of this PR) removed it, so SetAmbiguousWidth now writes DefaultCondition only when the user actually overrides the width. So an init() in util.go would be the no-op above, unless it hardcodes a value (more on that below).

2. Updating the dependency does not help. v0.0.21 autodetects the same way, it just as automatically decides double:

go-runewidth v0.0.21 (latest), LANG=ja_JP.UTF-8
runewidth.EastAsianWidth                  = true
runewidth.DefaultCondition.EastAsianWidth = true (after go-runewidth's own init)
runewidth.RuneWidth(U+03B1 alpha)         = 2

3. Issue 2 of #1124 is therefore still live on master. pty capture, LANG=ja_JP.UTF-8, buffer let αβγ = 0, ESC = 0x1b:

master   : '6mαESC[1;9HβESC[1;11HγESC[1;13HESC(BESC[mESC[38;5;239;'
branch/auto: '6mαESC[1;9HβESC[1;11HγESC[1;13HESC(BESC[mESC[38;5;239;'
branch/single: '6mαβγESC(BESC[mESC[38;5;239;48;5;236m ESC(BESC[mESC[97'
branch/double: '6mαESC[1;9HβESC[1;11HγESC[1;13HESC(BESC[mESC[38;5;239;'

master and -ambiguouswidth auto re-position two columns per rune; single writes the three runes contiguously. Default behaviour is unchanged.

Why a setting instead of a hardcoded = false: the correct value is a property of the terminal, not of the locale, and this thread contains both terminals. glibc 2.39 here reports narrow even in a CJK locale (locale built with localedef -i ja_JP -f UTF-8):

setlocale=ja_JP.UTF-8
wcwidth(U+03B1 alpha (Ambiguous)) = 1
wcwidth(U+0394 Delta (Ambiguous)) = 1
wcwidth(U+3042 hiragana (Wide)) = 2

vim/nano therefore assume one column, and @dmaluka reports that looking worse than micro's current double in their setup, which implies a terminal that draws these two columns wide, while yours draws them one. That is exactly the split vim's ambiwidth option exists for: hardcoding false fixes your terminal and misaligns theirs with no way back, and RUNEWIDTH_EASTASIAN=0 is only read at process start.

I am happy to flip the default to single (your UAX #11 quote, narrow when the context is not reliable) and keep double/auto for the other case, which makes your patch the out-of-the-box behaviour without removing the escape hatch. One-line change, say the word and I will push it.

Test status: TestSetAmbiguousWidth passes, and replacing the single assignment in SetAmbiguousWidth with _ = wide makes it fail (expected 5, actual 4):

FAIL	github.com/micro-editor/micro/v2/internal/util	0.005s

Not verified, stated as such: musl's wcwidth (no musl machine here), so the musl 1.2.5 claim in the PR description is untested by me, and I have not tested a terminal that actually renders ambiguous characters two columns wide, only the byte stream micro emits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants