Inserts when char_idx == gap_end can result in mangled buffer state

sminez commented 1 month ago

Describe the bug

In some circumstances (still trying to pin down the exact cause) it is possible to insert into a buffer and have the new text end up in the wrong place. This also causes undo of the edits in question to further mangle the buffer state.

Triggering this is quite tricky to do without a known case as it involves manipulating the internal state of the gap buffer in a particular way that sets things up for a following edit to start breaking things.

To Reproduce

So far the only reliable way I have of reproducing this is the exact case where I encountered it, but I suspect that this is the cause of some other odd behaviour I've seen in a few places as well. In all cases that I think are related, this involves opening an existing file and then making edits towards the end of the file (but not at the end of the file).

Test data

Edit 6,32 x/.*\n/ y/\s+-- / x/(.+)/ c/"$1"/
Edit 6,32 x/"(.+)"\s+--/ c/vec!["$1"],/
Edit 6,32 x/"(.+) \| (.+)"/ c/"$1", "$2"/
Edit 6,32 x/(.+)\n/ c/($1),\n/

b | buffer            -- switch to the buffer with the given ID ('buffer 5')
bn | buffer-next      -- switch to the next available open buffer in the buffer list
bp | buffer-prev      -- switch to the previous available open buffer in the buffer list
cd | change-directory -- change ad's working directory ('cd ../src')
db | delete-buffer    -- delete the active buffer as long as there are no pending changes
db! | delete-buffer!  -- delete the active buffer discarding all pending changes
echo                  -- display the given string in the status line ('echo hello, world!')
E | Edit              -- run an Edit command (See 'Running Edit Commands')
expand-dot            -- smart expand the current cursor position into a range
help                  -- display this help file
mark-clean            -- mark the current buffer as being clean to prevent saving changes
o | open              -- open the given file path in a new buffer ('open README.md')
pwd                   -- print the current editor working directory
q | quit              -- quit ad as long as there are no buffers with pending changes
q! | quit!            -- quit ad discarding all pending changes for open buffers
reload-buffer | Get   -- refresh the current buffer's content from the state of the file on disk
reload-config         -- reload the editor config file located at ~/.ad/init.conf
set                   -- set a config property ('set bg-color=#ebdbb2')
view-logs             -- open ad's internal logs in a new buffer
viewport-bottom       -- place the current line at the bottom of the window
viewport-center       -- place the current line at the center of the window
viewport-top          -- place the current line at the top of the window
wq | write-quit       -- save the current buffer to disk and exit, blocking if other buffers are dirty
wq! | write-quit!     -- save the current buffer to disk and exit, discarding other changes
w | write             -- save the current buffer to disk. (Blocked if the file has been modified on disk)
w! | write!           -- save the current buffer to disk ignoring external changes

Save the file contents above locally
Open the file in ad
From NORMAL mode type x@ to select and execute the first Edit command
See that the end of the buffer is mangled

Note that it is important that no other edits are made to the buffer. This is something to do with how the insert position relates to the current gap position in the underlying gap buffer I suspect.

Changing the first line to begin Edit 6,30 ... instead will run just fine. If you then manually start adding the expected quote you should see that it is inserting the quote before save on the second to last line that causes things to start breaking (with additional edits past that point making things worse). All I've been able to identify so far is that that character offset within the visible buffer is exactly the byte offset of self.gap_end for the underlying gap buffer. So I suspect that the issue is something to do with the implementation of char_to_byte_impl but that is not confirmed yet.

Expected behavior

Edits at any point within a buffer work.

Screenshots

This is the resulting buffer state after running the Edit command:

Edit 6,32 x/.*\n/ y/\s+-- / x/(.+)/ c/"$1"/
Edit 6,32 x/"(.+)"\s+--/ c/vec!["$1"],/
Edit 6,32 x/"(.+) \| (.+)"/ c/"$1", "$2"/
Edit 6,32 x/(.+)\n/ c/($1),\n/

"b | buffer"            -- "switch to the buffer with the given ID ('buffer 5')"
"bn | buffer-next"      -- "switch to the next available open buffer in the buffer list"
"bp | buffer-prev"      -- "switch to the previous available open buffer in the buffer list"
"cd | change-directory" -- "change ad's working directory ('cd ../src')"
"db | delete-buffer"    -- "delete the active buffer as long as there are no pending changes"
"db! | delete-buffer!"  -- "delete the active buffer discarding all pending changes"
"echo"                  -- "display the given string in the status line ('echo hello, world!')"
"E | Edit"              -- "run an Edit command (See 'Running Edit Commands')"
"expand-dot"            -- "smart expand the current cursor position into a range"
"help"                  -- "display this help file"
"mark-clean"            -- "mark the current buffer as being clean to prevent saving changes"
"o | open"              -- "open the given file path in a new buffer ('open README.md')"
"pwd"                   -- "print the current editor working directory"
"q | quit"              -- "quit ad as long as there are no buffers with pending changes"
"q! | quit!"            -- "quit ad discarding all pending changes for open buffers"
"reload-buffer | Get"   -- "refresh the current buffer's content from the state of the file on disk"
"reload-config"         -- "reload the editor config file located at ~/.ad/init.conf"
"set"                   -- "set a config property ('set bg-color=#ebdbb2')"
"view-logs"             -- "open ad's internal logs in a new buffer"
"viewport-bottom"       -- "place the current line at the bottom of the window"
"viewport-center"       -- "place the current line at the center of the window"
"viewport-top"          -- "place the current line at the top of the window"
"wq | write-quit"       -- "save the current buffer to disk and exit, blocking if other buffers are dirty"
"wq! | write-quit!"     -- "save the current buffer to disk and exit, discarding other changes"
"w | write"         -- save"save the current buffer to disk. (Blocked if the file has been modi"fied on disk)"
"w! | write!   " the current buffer to disk ignoring external changes"

The last two lines should have had the same edit made to them: quotes being placed around the text before and after the central -- delimiter.

sminez commented 1 month ago

I tried digging into at what point things break in the example failure described above and ended up seemingly tracking it down to inserting strings into the buffer once the gap is below a certain threshold. For the example in the regression test the break point is a gap size of 15 it seems? Forcing the gap to be resized at this point (a change in behaviour) fixes the issue, or at least, gets this specific regression test to pass. What I'm still not sure about is why this works and why 15 is the important value (the inserted string has length 11 so it's not a case of overflowing the gap).

My concern is that this "fix" is just kicking things down the line to show up again later. I'd prefer to fully understand why this is happening and be able to reproduce it with a simpler test case that is easier to reason about.

sminez commented 1 month ago

Closing for now as the issue hasn't resurfaced since committing the fix last week.

sminez / ad