simnalamburt / vim-mundo

:christmas_tree: Vim undo tree visualizer
https://simnalamburt.github.io/vim-mundo
GNU General Public License v2.0
792 stars 28 forks source link

Run __mundo__ autocmds earlier #92

Open BlueDrink9 opened 4 years ago

BlueDrink9 commented 4 years ago

I use some autocmds to set the default filetype for new buffers:

autocmd myVimrc BufNewFile * filetype detect | if &filetype == "" | setlocal ft=scratch | endif
" No names
autocmd myVimrc BufEnter {} if &filetype == "" | setlocal ft=scratch | endif
autocmd myVimrc BufEnter * if &filetype == "" | setlocal ft=conf

For some reason, the combination of the first and last autocmd in that list breaks mundo, causing the __mundo__ files to be opened as actual non-hidden new files, with ft=scratch.

Changing the MundoAug groups to BufNewFile instead of BufEnter fixes this:

autocmd BufNewFile __Mundo__ call s:MundoSettingsGraph()
autocmd BufNewFile __Mundo_Preview__ call s:MundoSettingsPreview()

This would mean re-entering an existing Mundo window wouldn't reapply the autocmd. That's potentially a benefit, since it is marginally more efficient if they only set local settings anyway (which can't be changed unless within the buffer), and wouldn't override user changes to buffer settings each time they re-enter it.

It also makes Mundo's ft setup etc more robust, since it's happening earlier in the setup process. It stops buggy vimrc or plugins from modifying the buffer if their autocmds fire first, for example.

BlueDrink9 commented 4 years ago

@simnalamburt do you have any thoughts on this?

simnalamburt commented 4 years ago

@BlueDrink9 OMG sorry to keep you extremely late.

We used BufNewFile in long time ago and changed it into BufEnter intensionally: https://github.com/simnalamburt/vim-mundo/pull/74

Well.. Since s:MundoSettingsGraph() and s:MundoSettingsPreview() is not that expensive function, fixing autoload/mundo.vim like this will both keep #74 and fix your issue:

diff --git a/autoload/mundo.vim b/autoload/mundo.vim
index 18009d9..3c437fe 100644
--- a/autoload/mundo.vim
+++ b/autoload/mundo.vim
@@ -533,8 +533,8 @@ augroup MundoAug
                     \ call s:MundoRenderPreview() |
                     \ call s:MundoStopRefreshTimer() |
                 \ endif |
-    autocmd BufEnter __Mundo__ call s:MundoSettingsGraph()
-    autocmd BufEnter __Mundo_Preview__ call s:MundoSettingsPreview()
+    autocmd BufNewFile,BufEnter __Mundo__ call s:MundoSettingsGraph()
+    autocmd BufNewFile,BufEnter __Mundo_Preview__ call s:MundoSettingsPreview()
     autocmd CursorHold,CursorMoved,TextChanged,InsertLeave *
                 \ call s:MundoRefresh()
 augroup END

But well.. I'm not sure if it's really a big deal to Execute __Mundo__ and __Mundo_Preview__ little bit early. Is it common practice to reload syntax highlightings and keymaps on both BufNewFile and BufEnter?

I made a draft PR of this patch, but I want to listen @dsummersl's opinion first. :)

BlueDrink9 commented 4 years ago

I see. #74 is definitely a reason to keep using bufenter. For me, the only thing that actually affects me is when the buffer filetype is set, so if you wanted to make the smallest possible change, separating out setting the filetype and only having that set with bufnewfile would work. I can't imagine a world where setting it on bufenter would be needed for anything.

BlueDrink9 commented 4 years ago

Alternatively, if it's just s:MundoSyntaxGraph() that #74 needs on bufenter, maybe that could be called separately on bufenter