Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

persistent-undo history is ignored by first session call to :n and :arglocal #5426

Closed
ivanwfr opened this issue Jan 1, 2020 · 6 comments
Closed

Comments

@ivanwfr
Copy link

ivanwfr commented Jan 1, 2020

First call to :arglocal or :n replaces persistent-undo history with current file content

To Reproduce
1 - Setup persistent-undo option :set undofile
2 - Edit and modify a file with this option activated
3 - Restart Windows gvim
4 - From the just-opened-empty-gvim-window
5 - Type ':n file'
6 - Type ':undolist'
... Result is a single command undo history list showing current file contents as if it was just fully written and saved.
7 - Type ':e!'
8 - Type ':undolist'
... Result is as it should be.

Expected behavior
The :undolist should be as expected for a previously persistent-undo edited file.

My current Workaround
Between Steps 4,5, just before calling ':n file'
4'' - Type ':f arglocal_issue_workaround'
This gives a file-name to the 'no-name first buffer' so that the implied ":update"
taking place before leaving it would make the persistent-undo processing happy
while it is busy doing nothing to update a 'nomodified' buffer we're leaving.
... This works fine for me, with no side-effect when I use a script that calls ':arglocal' as a first session command.

Screenshots
n.a.

Environment

  • Vim version:
    VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Dec 12 2019 13:30:17)
    MS-Windows 32-bit GUI version with OLE support
    Compiled by mool@tororo
    Huge version with GUI.

  • OS:
    Windows 7

  • Terminal:
    GUI

@chrisbra
Copy link
Member

chrisbra commented Jan 2, 2020

This happens because :next will eventually set curbuf in buflist_new (stack:

		#0  0x00005555555ca739 in buflist_new (ffname_arg=0x5555558fecf0 "/home/chrisbra/.vim/vimrc", sfname_arg=0x0, lnum=0, flags=3) at buffer.c:2156
		#1  0x00005555555cce4e in buflist_add (fname=0x5555558fecf0 "/home/chrisbra/.vim/vimrc", flags=3) at buffer.c:3453
		#2  0x00005555555c048d in alist_add (al=0x5555558db380 <global_alist>, fname=0x5555558fecf0 "/home/chrisbra/.vim/vimrc", set_fnum=2) at arglist.c:188
		#3  0x00005555555c03ba in alist_set (al=0x5555558db380 <global_alist>, count=1, files=0x5555558e9c00, use_curbuf=1, fnum_list=0x0, fnum_len=0) at arglist.c:157
		#4  0x00005555555c0d73 in do_arglist (str=0x5555558fc0e2 "~/.vim/vimrc", what=1, after=0, will_edit=1) at arglist.c:458
		#5  0x00005555555c176e in ex_next (eap=0x7fffffffd310) at arglist.c:721
		#6  0x000055555563df3e in do_one_cmd (cmdlinep=0x7fffffffd9a8, sourcing=0, cstack=0x7fffffffd500, fgetline=0x55555565183c <getexline>, cookie=0x0) at ex_docmd.c:2485
		#7  0x000055555563b48d in do_cmdline (cmdline=0x0, fgetline=0x55555565183c <getexline>, cookie=0x0, flags=0) at ex_docmd.c:975
		#8  0x00005555556cf0fe in nv_colon (cap=0x7fffffffdae0) at normal.c:3335
		#9  0x00005555556cb048 in normal_cmd (oap=0x7fffffffdbd0, toplevel=1) at normal.c:1088
		#10 0x0000555555827391 in main_loop (cmdwin=0, noexmode=0) at main.c:1510
		#11 0x00005555558266d4 in vim_main2 () at main.c:901
		#12 0x0000555555825dd6 in main (argc=3, argv=0x7fffffffddc8) at main.c:444

This causes later in do_ecmd, that otherfile() returns true, as Vim thinks we are editing the same file again, causing it to set the READ_KEEP_UNDO flag when loading the buffer (which skips loading the undofile).

I first thought, we should make otherfile_buf() return true, if the buffer is not loaded yet and while this fixes the problem, it introduces another one, as this will not reuse the old empty buffer and we will end up with two buffers with the same name, something like this

:ls!
:ls
  1 #    "~/.vim/vimrc"                 line 1
  2 %a   "~/.vim/vimrc"                 line 1

so I think it is better to use this patch to do_ecmd()

diff --git a/src/ex_cmds.c b/src/ex_cmds.c
index c8a93bc0a..7f0563790 100644
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -2866,7 +2866,8 @@ do_ecmd(
            new_name = NULL;
        set_bufref(&bufref, buf);

-       if (p_ur < 0 || curbuf->b_ml.ml_line_count <= p_ur)
+       if (!(curbuf->b_ml.ml_flags & ML_EMPTY) &&
+               (p_ur < 0 || curbuf->b_ml.ml_line_count <= p_ur))
        {
            // Save all the text, so that the reload can be undone.
            // Sync first so that this is a separate undo-able action.

@brammool what do you think? I can create a PR with a test, if you think this is the right approach.

@ivanwfr
Copy link
Author

ivanwfr commented Jan 2, 2020

@chrisbra,
My thanks for taking steps on the issue ;-)

@brammool
Copy link
Contributor

brammool commented Jan 2, 2020 via email

@ivanwfr
Copy link
Author

ivanwfr commented Jan 22, 2020

My workaround is sometimes messy:

  • When first called with some opened file, it would be saved with the fancy "arglocal_issue_workaround" file name which sucks the undolist.
  • I had to call :new to make it happen with an empty buffer that has nothing worth sucking...
  • But that is far from perfect as long as when a script uses that trick, it creates a extra window.

As I'm not hacking vim code myself since many years now, I would like to know who to beg for some effort in trying to solve this persistent undo issue.

@ivanwfr
Copy link
Author

ivanwfr commented Aug 19, 2020

Another workaround could be a clue for a solution;

  • :n file1 file2<cr>:n<cr>:rew<cr>

works only when there is more than one file in the argument list

The pipe version is less shaky (same thing .. no screen redraw)

  • :n file1 file2\|:n\|:rew<cr>

@ivanwfr
Copy link
Author

ivanwfr commented Aug 23, 2020

As it looks like I'm the only one to use :next while working with persistent undo and, as it makes no sense to have a persistent opened tickets, I'm going to to close it right now.
This way, I wont be tempted to look for some update that should have happened already from people who care to do what it takes for a good job.
After all, we can live with some sticky patches here and there and move on.

@ivanwfr ivanwfr closed this as completed Aug 23, 2020
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

No branches or pull requests

3 participants