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

Suppress autocmds less aggressively #578

Merged
merged 1 commit into from Apr 12, 2017
Merged

Suppress autocmds less aggressively #578

merged 1 commit into from Apr 12, 2017

Conversation

wincent
Copy link
Contributor

@wincent wincent commented May 5, 2016

This is the counterpart to a PR I just submitted to undotree (mbbill/undotree#61).

I noticed that my statusline doesn't update properly when using NERDTree to move between revisions of a file with go or gi (wincent/wincent#16). I established that this was because it was using 'eventignore' to suppress all autocmds, which in turn prevents the statusline from updating.

Commenting out the set eventignore=all line makes the failure to update go away, at the cost of firing more autocmds.

I considered adding an option for opting out of this behavior (eg. let g:NERDTreeEventignore=0 or something), or rearchitecting my statusline to use an approach like vim-airline does based on CursorMoved autocmds (see vim-airline/vim-airline#82; see also https://github.com/vim-airline/vim-airline/blob/30f078daf569e7d5e4f7829e39316387af349b41/plugin/airline.vim#L36-L50 for current implementation), but then realized that a simpler fix is to have NERDTree just disable only the autocmds that it uses instead of disabling all of them.

This is probably not enough to unbreak every bit of code in the world that depends on those autocmds, but it does at least unbreak my use case, because it allows my WinLeave autocmd to run and update the statusline.

This is the counterpart to a PR I just submitted to undotree
(mbbill/undotree#61).

I noticed that my statusline doesn't update properly when using NERDTree to move
between revisions of a file with `go` or `gi`
(wincent/wincent#16). I established that this was
because it was using `'eventignore'` to suppress all autocmds, which in turn
prevents the statusline from updating.

Commenting out the `set eventignore=all` line makes the failure to update go
away, at the cost of firing more autocmds.

I considered adding an option for opting out of this behavior (eg. `let
g:NERDTreeEventignore=0` or something), or rearchitecting my statusline to use
an approach like vim-airline does based on CursorMoved autocmds (see
vim-airline/vim-airline#82; see also
https://github.com/vim-airline/vim-airline/blob/30f078daf569e7d5e4f7829e39316387af349b41/plugin/airline.vim#L36-L50
for current implementation), but then realized that a simpler fix is to have
NERDTree just disable only the autocmds that it uses instead of disabling all of
them.

This is probably not enough to unbreak every bit of code in the world that
depends on those autocmds, but it does at least unbreak my use case, because it
allows my `WinLeave` autocmd to run and update the statusline.
wincent added a commit to wincent/wincent that referenced this pull request May 5, 2016
@wincent
Copy link
Contributor Author

wincent commented Apr 11, 2017

Friendly ping. Got any feedback on this PR? Thanks.

@PhilRunninger PhilRunninger merged commit 45f4d61 into preservim:master Apr 12, 2017
@bluz71
Copy link

bluz71 commented Apr 17, 2017

This change dramatically effects the speed of opening files via NERDtree for me (for the worse). Files take a second or more to load, where previously they used to load instantly.

This line in autoload/nerdtree.vim is the culprit:

set ei=BufEnter,BufLeave,VimEnter

I set my own statusline and I listen on VimEnter,WinEnter,BufWinEnter,InsertLeave events to change the statusline depending on normal/insert/visual or replace modes.

My slowness issue is fixed if I change the above line in autoload/nerdtree.vim to:

set ei=BufEnter,BufLeave,VimEnter,WinEnter

Notice the extra WinEnter.
I have no idea why this change effects the speed of opening files.

Previously in NERDtree it was:

set ei=all

Any chance of adding the WinEnter option to the current ei setting?

@wincent
Copy link
Contributor Author

wincent commented Apr 17, 2017

@bluz71: The reason things slow down when the WinEnter autocommand is not suppressed is that you have something expensive happening in your WinEnter autocommands. What plug-ins are you using?

Note that NERDTree itself does not make use of the WinEnter autommands, so it has no business suppressing those events and breaking other plug-ins that may rely on them being fired consistently. (It only uses BufEnter, BufLeave and VimEnter.)

@bluz71
Copy link

bluz71 commented Apr 17, 2017

I have my own function hooked up to WinEnter, one of the things that it did (apart from setting my statusline) was it auto-refreshed my NERDtree pane if I focussed back into the NERDtree pane.

Basically I called this function (via an autocmd for WinEnter):

function! NERDTreeRefresh()
    if &filetype == "nerdtree"
        silent exe substitute(mapcheck("R"), "<CR>", "", "")
    endif
end

That now is a very slow operation with this particular Git commit.
Why?

Do you know of any other way to auto-refresh the NERDtree, inexpensively, when focussing back into NERDtree?

I tried created a new autocmd, but this one is not firing at all:

autocmd WinEnter nerdtree call NERDTreeRefresh()

???

@bluz71
Copy link

bluz71 commented Apr 17, 2017

@wincent,

This crude autocmd exhibits the slowness I am now experiencing (easy for you to test):

autocmd WinEnter * if exists('b:NERDTree') | execute 'normal R' | endif

Without this Git commit it is quite fast.
With this Git commit it is very slow.
If adding WinEnter to the ei then it becomes quite fast again.

Basically I want a way to auto refresh NERDtree, I don't care how to achieve it.

Any ideas?

@wincent wincent deleted the eventignore branch April 17, 2017 15:00
@bluz71
Copy link

bluz71 commented Apr 17, 2017

I think I've fixed my issue in my vimrc.

This rule appears to be working AND with no NERDTree slowdown:

    autocmd BufEnter * call NERDTreeRefresh()

Basically I moved my NERDTreeRefresh out of WinEnter and into BufEnter. Functionality seems the same but with no speed issues.

@wincent
Copy link
Contributor Author

wincent commented Apr 18, 2017

Awesome. Glad that you sorted it out.

@lifecrisis
Copy link
Contributor

lifecrisis commented May 18, 2017

Greetings,

I haven't read this entire thread, but I will when I have the time. Since this pull request has just come to my attention, I wanted to leave my thoughts before further commits to this repository are made.

  1. I raised this issue some time ago in issue Why is ignoring autocommands necessary? #640. Please review my comment there. This pull request fixes that issue.
  2. @wincent, please note that you neglected to change the commentary above the function you modified. This is a critical oversight because it could confuse someone later.

I certainly agree with the change, and I'm glad to see it merged. I, too, thought the previous solution was overly aggressive.

The commit should be amended to change the function comment and to use the commit message to close issue #640. I hope @wincent and @PhilRunninger can work this out.

Thanks, guys!

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.

None yet

4 participants