From 7b16e44f534354385d5c566e54173b14a1c16d60 Mon Sep 17 00:00:00 2001 From: Junegunn Choi Date: Sat, 8 Aug 2026 19:10:51 +0900 Subject: [PATCH] Copy the item text in replace-query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ToRunes aliases the rune array, and the editing actions append into t.input in place when the cursor is not at the end, so the keystrokes edit the item. Non-ASCII items only, ASCII gets a fresh slice. printf '한글abcde\n' | fzf --bind 'ctrl-y:replace-query' ctrl-y, Left, BSpace, ctrl-u -> 한글abcee Runes and ToRunes are now documented read-only. A stale fold bit was the other symptom, letting the prefilter reject an item the general path matches. --- src/terminal.go | 4 +++- src/util/chars.go | 17 +++++++++++------ src/util/chars_test.go | 26 ++++++++++++++++++++++++++ test/test_core.rb | 13 +++++++++++++ 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/terminal.go b/src/terminal.go index 00f0b2bb..640f5be8 100644 --- a/src/terminal.go +++ b/src/terminal.go @@ -7417,7 +7417,9 @@ func (t *Terminal) Loop() error { case actReplaceQuery: current := t.currentItem() if current != nil { - t.input = current.text.ToRunes() + // ToRunes aliases the item text in rune mode, and the + // editing actions below append into t.input in place + t.input = append([]rune{}, current.text.ToRunes()...) t.cx = len(t.input) } case actFatal: diff --git a/src/util/chars.go b/src/util/chars.go index a2ddbd01..e9eb51c5 100644 --- a/src/util/chars.go +++ b/src/util/chars.go @@ -21,11 +21,9 @@ const ( type Chars struct { slice []byte // or []rune - // Written while the item is built and never after it reaches a matcher, - // so nothing reads these bits concurrently with a write. Prepend touches - // them, but only on the transient tokens inside transformItem, before - // item.text exists. trimLength* is kept out because TrimLength writes it - // lazily, long after that point. + // Only ever set, never cleared, so a reader racing a Prepend sees either + // the old or the new value and both are safe. trimLength* is kept out + // because TrimLength rewrites it. flags uint8 trimLengthKnown bool trimLength uint16 @@ -157,6 +155,8 @@ func ToChars(bytes []byte) Chars { return runesToChars(runes, mayFold) } +// RunesToChars adopts the caller's slice rather than copying it, so the caller +// must not keep mutating it. See Runes for why. func RunesToChars(runes []rune) Chars { mayFold := false for _, r := range runes { @@ -188,7 +188,10 @@ func (chars *Chars) MayFoldToAscii() bool { return chars.flags&flagMayFold != 0 } -// Runes returns the underlying rune slice, or nil if the text is kept as bytes. +// Runes returns the underlying rune slice, or nil if the text is kept as +// bytes. Read only. The result aliases the text, so writing to it would change +// the text without updating the cached fold bit, and the prefilter would then +// reject items it should match. Copy before mutating. func (chars *Chars) Runes() []rune { return chars.optionalRunes() } @@ -340,6 +343,8 @@ func (chars *Chars) ToString() string { return unsafe.String(unsafe.SliceData(chars.slice), len(chars.slice)) } +// ToRunes returns the text as runes. In rune mode the result aliases the text +// and must not be mutated, see Runes. In byte mode it is a fresh slice. func (chars *Chars) ToRunes() []rune { if runes := chars.optionalRunes(); runes != nil { return runes diff --git a/src/util/chars_test.go b/src/util/chars_test.go index c31f194e..5dd1ca7e 100644 --- a/src/util/chars_test.go +++ b/src/util/chars_test.go @@ -257,3 +257,29 @@ func TestMayFoldFlag(t *testing.T) { t.Error("Prepend of a foldable prefix must set the flag") } } + +// Runes and ToRunes alias the text in rune mode, so a consumer that mutates +// what they return changes the text without updating the cached fold bit. This +// pins the aliasing so the read-only contract on those methods is not silently +// dropped later. +func TestRuneSlicesAliasTheText(t *testing.T) { + chars := ToChars([]byte("한글abc")) + runes := chars.Runes() + if runes == nil { + t.Fatal("expected rune mode") + } + if &runes[0] != &chars.ToRunes()[0] { + t.Error("Runes and ToRunes should return the same backing array") + } + if chars.MayFoldToAscii() { + t.Fatal("baseline should not be foldable") + } + // Demonstrates why callers must copy: the flag does not follow the text. + runes[0] = 'e' + if chars.MayFoldToAscii() { + t.Error("flag unexpectedly updated") + } + if got := chars.ToString(); got != "e글abc" { + t.Errorf("expected the write to reach the text, got %q", got) + } +} diff --git a/test/test_core.rb b/test/test_core.rb index a8ef16a4..f32e24b1 100644 --- a/test/test_core.rb +++ b/test/test_core.rb @@ -479,6 +479,19 @@ class TestCore < TestInteractive tmux.until { |lines| assert_equal '> 10', lines[-1] } end + def test_bind_replace_query_does_not_mutate_item + tmux.send_keys "echo '한글abcde' | #{fzf('--bind=ctrl-j:replace-query,ctrl-o:clear-query')}", :Enter + tmux.until { |lines| assert_equal ' 1/1', lines[-2] } + tmux.send_keys 'C-j' + tmux.until { |lines| assert_equal '> 한글abcde', lines[-1] } + # Editing away from the end used to write into the item itself + tmux.send_keys :Left, :BSpace + tmux.until { |lines| assert_equal '> 한글abce', lines[-1] } + tmux.send_keys 'C-o' + tmux.until { |lines| assert_equal '>', lines[-1] } + tmux.until { |lines| assert_equal '> 한글abcde', lines[-3] } + end + def test_select_all_deselect_all_toggle_all tmux.send_keys "seq 100 | #{fzf('--bind ctrl-a:select-all,ctrl-d:deselect-all,ctrl-t:toggle-all --multi')}", :Enter tmux.until { |lines| assert_equal ' 100/100 (0)', lines[-2] }