Redo #258, Revert #302 and Fix #235 #303

svanharmelen · 2016-09-17T19:38:50Z

The first two commits are to redo/undo earlier changes that were made related to this issue, that turn out to properly fix the problem.

The third commit is the hack/fix that fixes the problem. Yet I wonder if this is an acceptable fix, but if not hopefully it will help us get there.

mattn · 2016-09-18T13:16:04Z

merged https://github.com/ctrlpvim/ctrlp.vim/commits/master

Thanks.

mattn · 2016-09-18T13:36:16Z

Well, When the cursor on the NERTree and typeing CtrlP, new buffer is opended at NERDTree. Is this right?

mattn · 2016-09-18T13:55:28Z

I think this fix is not perfect.

mattn · 2016-09-18T14:08:50Z

I looked this. I guess we shouldn't revert #302.

svanharmelen · 2016-09-18T14:29:53Z

Like I said in the comment of the PR this is more of a hack that a perfect fix. So I didn't expect you to merge it right away.

Well, When the cursor on the NERTree and typeing CtrlP, new buffer is opended at NERDTree. Is this right?

No, this is indeed not right.

I looked this. I guess we shouldn't revert #302.

I don't really understand that patch as it doesn't seem to fix the problems I'm seeing. But I do agree this current fix is not complete and/or good enough.

So we still need to work more on this issue to get to a proper fix 😞 Will invest some more time this evening...

mattn · 2016-09-18T14:36:46Z

Okay, Sorry I didn't check before merging. Please don't mind. #302 is required fix even though this problems. So I'll merge this again. Let's think fix again. Thanks.

See #303

mattn · 2016-09-18T14:44:24Z

Then, please explain how to reproduce your issue using NERDTree again.

svanharmelen · 2016-09-18T14:51:50Z

Okay, Sorry I didn't check before merging. Please don't mind. #302 is required fix even though this problems. So I'll merge this again

Check!

Then, please explain how to reproduce your issue using NERDTree again.

Yes, let me create a minimal .vimrc and send you that together with a small gif showing the problem so you can reproduce.

In the mean time I suggest to revert commit 063b538 until we have a proper fix.

See #303

mattn · 2016-09-18T14:55:42Z

Thanks!

svanharmelen · 2016-09-18T15:13:38Z

So this is a .vimrc that you can use to reproduce the problem:

call plug#begin('~/.config/nvim/plugged')

" Add plugins
Plug 'ctrlpvim/ctrlp.vim'
Plug 'scrooloose/nerdtree'
Plug 'xuyuanp/nerdtree-git-plugin'

call plug#end()

" Set this to 500ms to trigger the nerdtree-git-plugin to update
" (which is the real root cause of this issue).
set updatetime=500

Now with this .vimrc (and of course after you have reverted commit 063b538):

Make a dir with two test files
Open vim in this dir
Type :NERDTree
Open one of your test files in your main window
Press <C-p>
Wait for at least 500ms
Select the other test file and press <CR>

The file should now be opened wrongly in the NERDTree window instead of the main window.

See the animated gif below as an example:

svanharmelen · 2016-09-18T15:22:40Z

And some more additional info... After those 500ms waiting the updatetime expires and triggers a CursorHold event which triggers the following code in the nerdtree-git-plugin plugin:

augroup nerdtreegitplugin
    autocmd CursorHold * silent! call s:CursorHoldUpdate()
augroup END
" FUNCTION: s:CursorHoldUpdate() {{{2
function! s:CursorHoldUpdate()
    if g:NERDTreeUpdateOnCursorHold != 1
        return
    endif

    if !g:NERDTree.IsOpen()
        return
    endif

    let l:winnr = winnr()
    call g:NERDTree.CursorToTreeWin()
    call b:NERDTree.root.refreshFlags()
    call NERDTreeRender()
    exec l:winnr . 'wincmd w'
endfunction

Which in turn causes this issue...

mattn · 2016-09-18T15:27:08Z

Try this

function! CtrlP_OpenAtCenter(...)
  " find free window
  for n in range(0, bufnr('$'))
    if buflisted(n)
      exe bufwinnr(n) . 'wincmd w'
      break
    endif
  endfor
  call call('ctrlp#acceptfile', a:000)
endfunction

let g:ctrlp_open_func = {
\  'arg_type': 'dict',
\  'files': 'CtrlP_OpenAtCenter',
\}

mattn · 2016-09-18T15:30:06Z

If this works expected, I'll try to add new option to skip window to open new buffer.

svanharmelen · 2016-09-18T15:39:44Z

Added it to my test .vimrc so it now looks like this:

call plug#begin('~/.config/nvim/plugged')

" Add plugins
Plug 'ctrlpvim/ctrlp.vim'
Plug 'scrooloose/nerdtree'
Plug 'xuyuanp/nerdtree-git-plugin'

call plug#end()

" Set this to 500ms to trigger the nerdtree-git-plugin to update,
" which actually is the root cause of this problem.
set updatetime=500

function! CtrlP_OpenAtCenter(...)
  " find free window
  for n in range(0, bufnr('$'))
    if buflisted(n)
      exe bufwinnr(n) . 'wincmd w'
      break
    endif
  endfor
  call call('ctrlp#acceptfile', a:000)
endfunction


let g:ctrlp_open_func = {
\  'arg_type': 'dict',
\  'files': 'CtrlP_OpenAtCenter',
\}

And this indeed seems to work and seems to fix the problem! 🎉

Sorry for not understanding that comment before. But I guess we should build this in CtrlP itself right? So users should not have to add this to their .vimrc in order to fix it?

mattn · 2016-09-18T15:42:04Z

Sorry for not understanding that comment before. But I guess we should build this in CtrlP itself right? So users should not have to add this to their .vimrc in order to fix it?

Yes, but maybe it will be new option.

svanharmelen · 2016-09-18T15:48:31Z

Hmm... Sorry but I continued with some additional testing of other cases, and there I still see the same issues. So let me cleanup some caches and run some proper tests to really make sure this indeed fixes the problem.

Will get back to you shortly...

mattn · 2016-09-18T15:51:09Z

Please explain how to reproduce.

mattn · 2016-09-18T16:08:53Z

function! CtrlP_OpenAtCenter(...)
  " find free window
  for n in range(0, winnr('$'))
    if buflisted(winbufnr(n))
      exe n . 'wincmd w'
      break
    endif
  endfor
  call call('ctrlp#acceptfile', a:000)
endfunction

let g:ctrlp_open_func = {
\  'arg_type': 'dict',
\  'files': 'CtrlP_OpenAtCenter',
\}

This seems better.

svanharmelen · 2016-09-18T18:25:26Z

No, this patch/fix doesn't work. To reproduce please do the following:

Copy this into your .vimrc:

call plug#begin('~/.config/nvim/plugged')

" Add plugins
Plug 'ctrlpvim/ctrlp.vim'
Plug 'scrooloose/nerdtree'
Plug 'xuyuanp/nerdtree-git-plugin'

call plug#end()

" Set this to 500ms to trigger the nerdtree-git-plugin to update,
" which actually is the root cause of this problem.
set updatetime=500

" For testing make sure we alway just open in the same window
let g:ctrlp_switch_buffer = 0

function! CtrlP_OpenAtCenter(...)
  " find free window
  for n in range(0, winnr('$'))
    if buflisted(winbufnr(n))
      exe n . 'wincmd w'
      break
    endif
  endfor
  call call('ctrlp#acceptfile', a:000)
endfunction

let g:ctrlp_open_func = {
\  'arg_type': 'dict',
\  'files': 'CtrlP_OpenAtCenter',
\}

Then take the following steps:

Make a dir with two test files
Open vim in this dir
Type :NERDTree
Open the first test file in your main window
Type :vs
Type <C-w> and then lto select the left most window
Press <C-p>
Wait for at least 500ms
Open the second test file and press <CR>

If you look closely you'll notice that the second file will be opened in the left window instead of the active right window. It doesn't open in the NERDTree window anymore, so that is good. But it's still not the expected behaviour.

Example:

svanharmelen · 2016-09-18T18:36:57Z

The previous comment shows how to reproduce the first kind of issue (that the wrong window is selected). But now it even gets weirder when you just add this line to your .vimrc:

let g:ctrlp_cmd = ' CtrlPMRU'

So to do another test, please copy this into your .vimrc:

call plug#begin('~/.config/nvim/plugged')

" Add plugins
Plug 'ctrlpvim/ctrlp.vim'
Plug 'scrooloose/nerdtree'
Plug 'xuyuanp/nerdtree-git-plugin'

call plug#end()

" Set this to 500ms to trigger the nerdtree-git-plugin to update,
" which actually is the root cause of this problem.
set updatetime=500

" For testing make sure we alway just open in the same window
let g:ctrlp_switch_buffer = 0

" Same test but then with `CtrlPMRU`
let g:ctrlp_cmd = 'CtrlPMRU'

function! CtrlP_OpenAtCenter(...)
  " find free window
  for n in range(0, winnr('$'))
    if buflisted(winbufnr(n))
      exe n . 'wincmd w'
      break
    endif
  endfor
  call call('ctrlp#acceptfile', a:000)
endfunction

let g:ctrlp_open_func = {
\  'arg_type': 'dict',
\  'files': 'CtrlP_OpenAtCenter',
\}

Again follow the same steps:

Make a dir with two test files
Open vim in this dir
Type :NERDTree
Open the first test file in your main window
Type :vs
Type and then lto select the left most window
Press
Wait for at least 500ms
Open the second test file and press

But now, you'll see that it actually still selects the NERDTree window to open the second test file in. So clearly adding the custom function to the .vimrc in combination with the code currently in master, does not solve the issues 😞

Example:

svanharmelen · 2016-09-19T06:33:55Z

@mattn last night I thought about this problem some more, and I realised that I was wrong. I no longer think we should fix this problem in CtrlP!

This problem is occurring not only when using CtrlP, but also when you open a quickfix or help window (or any other kind of special window). The only difference is that it is much less clear in those cases, because the only thing you'll notice is that your focus is wrong. So you'll select the proper window again and continue your work. And with CtrlP not only your focus is wrong, but also the content is in an unexpected location.

So if we only fix it in CtrlP it's not actually fixed. It's only fixed for CtrlP, but not for the other cases. Long story short, I will try to get in contact with the two plugin maintainers that I currently know cause this problem and will see if I can get to a solution with them...

Sander van Harmelen added 3 commits September 17, 2016 21:33

Redo #258

3cf152f

Revert #302

395d7f4

Fix #235

063b538

svanharmelen mentioned this pull request Sep 17, 2016

bug (?): with NERDtree open, files are always opened in the left/top most split #235

Closed

mattn closed this Sep 18, 2016

mattn added a commit that referenced this pull request Sep 18, 2016

Merge #302 again

577836f

See #303

mattn added a commit that referenced this pull request Sep 18, 2016

Revert 1d53c4b

0556da8

See #303

This was referenced Sep 20, 2016

Do not update when a special buffer is selected Xuyuanp/nerdtree-git-plugin#63

Merged

Do not update when a special buffer is selected ryanoasis/vim-devicons#166

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redo #258, Revert #302 and Fix #235 #303

Redo #258, Revert #302 and Fix #235 #303

svanharmelen commented Sep 17, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016

svanharmelen commented Sep 18, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016 •

edited

Loading

svanharmelen commented Sep 18, 2016

mattn commented Sep 18, 2016

svanharmelen commented Sep 18, 2016 •

edited

Loading

svanharmelen commented Sep 18, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016

svanharmelen commented Sep 18, 2016

mattn commented Sep 18, 2016

svanharmelen commented Sep 18, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016

svanharmelen commented Sep 18, 2016

svanharmelen commented Sep 18, 2016

svanharmelen commented Sep 19, 2016

Redo #258, Revert #302 and Fix #235 #303

Redo #258, Revert #302 and Fix #235 #303

Conversation

svanharmelen commented Sep 17, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016

svanharmelen commented Sep 18, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016 • edited Loading

svanharmelen commented Sep 18, 2016

mattn commented Sep 18, 2016

svanharmelen commented Sep 18, 2016 • edited Loading

svanharmelen commented Sep 18, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016

svanharmelen commented Sep 18, 2016

mattn commented Sep 18, 2016

svanharmelen commented Sep 18, 2016

mattn commented Sep 18, 2016

mattn commented Sep 18, 2016

svanharmelen commented Sep 18, 2016

svanharmelen commented Sep 18, 2016

svanharmelen commented Sep 19, 2016

mattn commented Sep 18, 2016 •

edited

Loading

svanharmelen commented Sep 18, 2016 •

edited

Loading