Skip to content

fix(textinput): account for horizontal scroll offset in Cursor()#1005

Open
guptarohit wants to merge 1 commit into
charmbracelet:mainfrom
guptarohit:fix/textinput-cursor-scroll-offset
Open

fix(textinput): account for horizontal scroll offset in Cursor()#1005
guptarohit wants to merge 1 commit into
charmbracelet:mainfrom
guptarohit:fix/textinput-cursor-scroll-offset

Conversation

@guptarohit

Copy link
Copy Markdown

Fixes #1001.

Cursor() computed its X from the absolute Position(), but when the value
overflows the configured width the input scrolls horizontally. View() already
renders the cursor at the scroll-adjusted column (m.pos - m.offset); Cursor()
did not, so the hardware cursor (SetVirtualCursor(false)) landed at the wrong
column once scrolled.

This aligns Cursor() with View() by using max(0, m.pos-m.offset) + promptWidth.

Added TestCursorXAccountsForScrollOffset. Note: I used a column-accurate
assertion with the cursor scrolled into the middle of the viewport — the
original issue's snippet asserted X > width, which ignores the prompt width and
would fail even after a correct fix at end-of-input.

@guptarohit
guptarohit force-pushed the fix/textinput-cursor-scroll-offset branch 2 times, most recently from 0147dbc to 22aecc0 Compare June 20, 2026 00:47
@guptarohit
guptarohit force-pushed the fix/textinput-cursor-scroll-offset branch from 22aecc0 to 9e5d53c Compare June 20, 2026 08:40
@guptarohit
guptarohit marked this pull request as ready for review June 20, 2026 13:24
@thomas-mangin

Copy link
Copy Markdown

Thanks for looking into my report. Moving the cursor into the middle of the viewport is what exposes the problem, as at the end, the width clamp hides it.

The patch currently counts runes but the test expects to count cells, so it only passes for ASCII (m.pos/m.offset are rune indices)

  • current patch (runes): xOffset := max(0, m.pos-m.offset) + promptWidth
  • test's expected value (cells): lipgloss.Width(m.promptView()) + lipgloss.Width(string(m.value[m.offset:m.pos]))

Measuring the visible prefix in cells fixes it (w is already lipgloss.Width in Cursor()):

xOffset := w(string(m.value[m.offset:m.pos])) + promptWidth

A CJK row locks it in:
m.SetValue(strings.Repeat("世", 30)) // 2 cells/rune, so rune count != column

I used Claude to verify against v2.1.0 (width 20, prompt > , cursor mid-scroll): the rune formula gives 7 where View() renders column 12; the cell formula gives 12. So the current fix fails the test on that input.

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.

v2: textinput Cursor() X ignores scroll offset when input overflows width

2 participants