[vim/vim] `ex_undojoin()` sets `b_u_curhead` for no reason (which causes bugs) (#1390)
Matthew Malcomson
As far as I can see, setting curbuf->b_u_curhead in ex_undojoin() provides no benefit and introduces some bugs.
ex_undojoin() sets curbuf->b_u_curhead at the same time as setting curbuf->b_u_synced to FALSE. When the undo state is synced with u_sync(), both b_u_curhead is set to NULL and b_u_synced is set to TRUE (unless the undo list is corrupt and u_get_headentry returns NULL when called in u_getbot()).
Throughout the source code there are 3 places where b_u_curhead is used without resetting b_u_synced to TRUE, these are bugs 1 and 2 below, and what causes using undojoin twice in a function to give the error E790 undojoin is not allowed after undo.
There is only one place where b_u_synced is set to TRUE without setting b_u_curhead back to NULL, this is bug 3 below.
Overall I think not setting b_u_curhead in ex_undojoin() would fix these bugs (though they could be fixed in other ways) and would not cause any regressions.
The three bugs this introduces are:
1 :undojoin | redo
Essentially swaps the order of things
Test
With the file
test.txt
first line
second line
Run the following:
vim -N -u NONE test.txt
ixx<esc>
:undojoin | redo<CR>
Observe
leaves the buffer as it was opened
running undo puts the x's back into the buffer.
cannot go backwards in time from there -- undo history is false
2 :undojoin | write breaks the :earlier 1f command
Test
open a new file
vim -N -u NONE test.txt
first write to the file<esc>
:w<CR>
osecond write to the file<esc>
osecond line not written<esc>
:undojoin | w<CR>
:earlier 1f<CR>
Observe in the buffer
first write to the file
second write to the file
But this text was never written to disk.
3 Interrupt after :undojoin
Artificially send an interrupt during u_savecommon() after calling
ex_undojoin().
This leaves the undo tree with a superfluous branch, that hasn't
actually been undone but is there as if it had been.
With the file
orig_vim_debug_this.txt
Test line for actions
yy
:put
attach gdb to nvim here -- run session provided below
:undojoin | delete
artificial Ctrl-c in gdb in u_savecommon() for delete
Observe in the buffer
The second line was not removed
Pressing 'u' does not remove the line -- vim thinks the end of the
changelist has been reached.
Pressing 'Ctrl-r' removes the line -- vim now thinks it is a change we
have already undone.
If we make another change, that percieved available redo is moved to
an alternate branch.
vim [19:36:50] % gdb src/vim
Reading symbols from src/vim...(no debugging symbols found)...done.
(gdb) set pagination off
(gdb) python
>import subprocess as sp
>matching_processes = []
>for pid in [val.decode('utf-8') for val in
> sp.check_output(['pgrep', 'vim']).splitlines()]:
> with open('/proc/{}/cmdline'.format(pid), 'r') as infile:
> cur_cmdline = infile.read()
> if 'orig_vim_debug_this' in cur_cmdline:
> matching_processes.append(pid)
>
>if len(matching_processes) == 1:
> gdb.execute('attach {}'.format(matching_processes[0]))
>else:
> print(matching_processes)
>end
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
0x00007fe462e37093 in __select_nocancel () from /usr/lib/libc.so.6
(gdb) break ex_undojoin
Breakpoint 1 at 0x58d9f0
(gdb) command 1
Type commands for breakpoint(s) 1, one per line.
End with a line saying just "end".
>silent
>set variable $savecommon_shouldbreak = 1
>cont
>end
(gdb) break u_savecommon if $savecommon_shouldbreak
Breakpoint 2 at 0x58b840
(gdb) command 2
Type commands for breakpoint(s) 2, one per line.
End with a line saying just "end".
>set variable $savecommon_shouldbreak = 0
>end
(gdb) cont
Continuing.
Breakpoint 2, 0x000000000058b840 in u_savecommon ()
(gdb) set variable got_int = 1
(gdb) cont
Continuing.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
Bram Moolenaar
Thanks. You are right, b_u_curhead should only be set after undo.