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

Fix E1312 when quitting a window #875

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

yousong
Copy link

@yousong yousong commented Feb 29, 2024

The idea is taken from README.md of MattLombana/dotfiles

Fixes #851

The idea is taken from README.md of MattLombana/dotfiles

Fixes preservim#851
@raven42
Copy link
Collaborator

raven42 commented Feb 29, 2024

I've added a couple more changes to this PR to address a couple other areas of concern. It should now handle vim-9 with the newer handling, but pre-vim-9 will still behave as the old method.

I've tested the following scenaios

autoclose_netrw=0 test cases

autoclose_netrw=0, single file, single window

In this scenario, open a single file in a single window. Then with Tagbar open and NERDTree open, hit the :q from the main window.

  • vim-8.1: NERDTree window only remaining window
  • vim-9.1: NERDTree window only remaining window

autoclose_netrw=0, single file, multi-window

Open a single file, split the window, then open Tagbar and NERDTree. Then hit :q from the one of the split windows

  • vim-8.1: the split closed leaving all other windows open
  • vim-9.1: the split closed leaving all other windows open (this was previously failing with my last attempt)

autoclose_netrw=0, multi-file, single-window

Open multiple files in different buffers, but only a single window. Then open Tagbar and NERDTree. Then hit :q from the main window.

  • vim8.1: File window closes, and Tagbar window closes leaving the netrw (NERDTree) window open but both files as active buffers. This is a behavioral bug with current tagbar code it looks like
  • vim9.1: File window closes, and Tagbar window closes leaving the netrw (NERDTree) window open but both files as active buffers. Still a bug, but same behavior as vim8.1 at least

autoclose_netrw=0, multi-file, single-window, one file edited but not written

Open multiple files in different buffers, but only a single window. Then open Tagbar and NERDTree. Next edit one of the files, but don't write it. Then hit :q from the main window.

  • vim8.1: File window closes, and Tagbar window closes leaving the netrw (NERDTree) window open but both files as active buffers. This is a behavioral bug with current tagbar code it looks like
  • vim9.1: File window closes, and Tagbar window closes leaving the netrw (NERDTree) window open but both files as active buffers. Still a bug, but same behavior as vim8.1 at least

autoclose_netrw=1 test cases

autoclose_netrw=1, single file, single window

In this scenario, open a single file in a single window. Then with Tagbar open and NERDTree open, hit the :q from the main window.

  • vim-8.1: all windows close and vim quits
  • vim-9.1: all windows close and vim quits

autoclose_netrw=1, single file, multi-window

Open a single file, split the window, then open Tagbar and NERDTree. Then hit :q from the one of the split windows

  • vim-8.1: the split closed leaving all other windows open
  • vim-9.1: the split closed leaving all other windows open (this was previously failing with my last attempt)

autoclose_netrw=1, multi-file, single-window

Open multiple files in different buffers, but only a single window. Then open Tagbar and NERDTree. Then hit :q from the main window.

  • vim8.1: VIM warns that there is still an open file and prompts 'Are you sure' about closing. If y chosen, then vim quits, if n chosen, then main window closes, tagbar window closes, and netrw window is left open with both files in buffer list. This looks like a behavioral bug.
  • vim9.1: No warning seen, main window closes, tagbar window closes, netrw window is left with only one file still in the buffer list. We have a definite change in behavior here which we may not want to go with.

autoclose_netrw=1, single-file, single-window, file edited but not written

Open a single file. Then open Tagbar and NERDTree. Next edit the file, but don't write it. Then hit :q from the main window.

  • vim8.1: VIM warns that the file is not written and asks if you want to write it. If y chosen, file is written and vim quits. If n is chosen, file is not written and vim quits. If c (cancel) chosen, then main file window remains, tagbar window closes, netrw window closes
  • vim9.1: No warning seen, main window closes, tagbar window closes, netrw window left open with file buffer still open unwritten. Again a change in behavior that I don't know we can go with.

Summary

This is definitely better, however in the cases where VIM would normally prompt a warning about something (multiple buffers, unwritten changes, etc), we have a break in functionality where we no longer see the prompt and windows are closed leaving VIM in an odd state.

I think we can build on this though and see if we can address these remaining items, maybe even fix the undesired behavior on vim-8.x as well.

try
try
quit
if has('patch-9.0.907')
call timer_start(50, {-> execute('q', 'silent!') })
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why different wait time argument for the 3 timer_start call? I think maybe what matters is where the command is going to be invoked, and the wait time can be all 0.

Copy link
Collaborator

@raven42 raven42 Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call timer_start() will return immediately after the call is queued. But there can be different background threads that will actually execute the queued call. So the delay is in there to allow for the previously queued call time to execute and complete before starting the next one. It is kind of a soft serialization of the queued calls so they aren't trying to all execute at the same time.

What we want to avoid

The par section will execute in parallel, and we don't have control over the order in which the threads execute.

sequenceDiagram
    participant main
    create participant only_window as s:HandleOnlyWindow()
    main->>only_window: call
    create participant close as timer_start(0)
    only_window-->>close: call timer_start(0)
    create participant bdelete as timer_start(0)
    only_window-->>bdelete: call timer_start(0)
    create participant quit as timer_start(0)
    only_window-->>quit: call timer_start(0)
    destroy only_window
    only_window->>main: return

    par
        note over bdelete: execute('noautocmd keepalt bdelete ' . tagbarwinnr)
    and
        note over close: s:CloseWindow()
    and
        note over quit: execute('q', 'silent!')
    end
    destroy close

    close->>main: done

    destroy bdelete
    bdelete->>main: done

    destroy quit
    quit->>main: done
Loading

Adding delay in timer

In this instance, we add the delays to ensure there is time for the other tasks to complete in proper order.

sequenceDiagram
    participant main
    create participant only_window as s:HandleOnlyWindow()
    main->>only_window: call
    create participant close as timer_start(10)
    only_window-->>close: call timer_start(10)
    create participant bdelete as timer_start(20)
    only_window-->>bdelete: call timer_start(20)
    create participant quit as timer_start(50)
    only_window-->>quit: call timer_start(50)
    destroy only_window
    only_window->>main: return

    note over close: s:CloseWindow()
    destroy close
    close->>main: done

    note over bdelete: execute('noautocmd keepalt bdelete ' . tagbarwinnr)
    destroy bdelete
    bdelete->>main: done

    note over quit: execute('q', 'silent!')
    destroy quit
    quit->>main: done
Loading

@plenaerts
Copy link

This problem still exists, but this thread is not moving anymore. Is there a temporary solution possible before an optimal solution can be decided on?

@raven42
Copy link
Collaborator

raven42 commented Dec 11, 2024

This problem still exists, but this thread is not moving anymore. Is there a temporary solution possible before an optimal solution can be decided on?

Due to the behavioral changes seen with this PR, I am extremely hesitant to merge this in. Especially in the case of a modified buffer. The behavior in this PR with regard to a modified buffer can result in data being lost if not handled correctly.

Instead we can see the correct behavior and consistent behavior if :qa is used to quit all. It may be better to just get in the habit of using this instead.

I've looked at this off and on as I have time, but I just don't have a lot of time to spend digging into this one. If someone else wants to pick it up, I can help review/test.

@plenaerts
Copy link

Instead we can see the correct behavior and consistent behavior if :qa is used to quit all. It may be better to just get in the habit of using this instead.

That seems to work perfectly, yes. I guess communicating this workaround would be useful. Would somewhere here be a good place?

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.

vim-9.0: The autoclose functionality is not behaving correctly
3 participants