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

Cursor moves back to beginning of line when exiting :FZF popup on Vim 9 (after #209) #224

Closed
cxw42 opened this issue Aug 19, 2023 · 24 comments · Fixed by #229 or #230
Closed

Cursor moves back to beginning of line when exiting :FZF popup on Vim 9 (after #209) #224

cxw42 opened this issue Aug 19, 2023 · 24 comments · Fixed by #229 or #230
Assignees
Labels

Comments

@cxw42
Copy link
Member

cxw42 commented Aug 19, 2023

Hi, recently I have an issue with editorconfig-vim after this PR merged.

test.vimrc:

set nocompatible

call plug#begin()
Plug 'editorconfig/editorconfig-vim'
Plug 'junegunn/fzf', { 'dir': '~/.fzf', 'do': './install --all' }
call plug#end()

autocmd FileType c EditorConfigReload

.editorconfig:

root = true

[*]
tab_width = 8

test.c:

#include <stdio.h>

int main() {
  int x = 1;
  printf("%d\n", x);
  return 0;
}

How to reproduce the problem:

  • Run vim -u test.vimrc test.c
  • Place the cursor anywhere except the first column
  • Run :FZF
  • Press Esc to close the popup
  • The cursor goes to the first column of line 1

I couldn't reduce test.vimrc further as I don't know much about popup windows. I've also run git bisect, and it pointed e269673.

Originally posted by @yous in #209 (comment)

@cxw42
Copy link
Member Author

cxw42 commented Aug 19, 2023

@yous thanks for reporting! What is the expected behaviour? What did it used to do that it doesn't any more?

Could you provide me the command-line or script you used for git bisect? Or did you manually test and use good/bad?

Much appreciated!

@yous
Copy link
Contributor

yous commented Aug 19, 2023

The expected behavior is, after I press Esc, the cursor should go back to the previous location. But currently, the cursor goes to the first line after the popup window closed, so screen scrolls to the top.

I ran git bisect good / git bisect bad manually.

@cxw42 cxw42 added the Bug label Aug 19, 2023
@cxw42
Copy link
Member Author

cxw42 commented Aug 19, 2023

Thanks for the details!

@midgleyc
Copy link

midgleyc commented Nov 7, 2023

I also have this problem. As a tiny bit more information, it happens on any pop-up specified by fzf (e.g. :W, :Files), and it goes to the top of the file when the pop-up opens, before you press escape to close. The cursor didn't used to move at all.

@cxw42
Copy link
Member Author

cxw42 commented Nov 18, 2023

@yous @midgleyc which Vim and FZF versions? I haven't been able to repro yet. (But I'm using set rtp+= rather than vim-plug, so I need to try that next.)

VIM - Vi IMproved 8.1 (2018 May 18, compiled Oct 16 2023 18:14:13)
Included patches: 1-213, 1840, 214-579, 1969, 580-1854, 1857, 1855-1857, 1331, 1858, 1858-1859, 1873, 1860-1969, 1992, 1970-1992, 2010, 1993-2269, 3612, 3625, 3669, 3741, 1847

and fzf 0.44.1.

@yous
Copy link
Contributor

yous commented Nov 18, 2023

I can reproduce in WSL with these versions:

$ vim --version
VIM - Vi IMproved 9.0 (2022 Jun 28, compiled Nov 11 2023 17:59:33)
Included patches: 1-2100
$ fzf --version
0.44.1 (d7d2ac3)

@midgleyc
Copy link

midgleyc commented Nov 18, 2023

Also WSL, with:

$ vim --version
VIM - Vi IMproved 9.0 (2022 Jun 28, compiled May 10 2022 08:40:37)
Included patches: 1-749
$ fzf --version
0.29 (devel)

And the latest versions of fzf.vim, installed through Plug:

Plug 'junegunn/fzf', { 'do': { -> fzf#install() } }
Plug 'junegunn/fzf.vim' " fuzzy finder

@cxw42
Copy link
Member Author

cxw42 commented Nov 19, 2023

Thanks for the quick responses! I'll try Vim 9.

@cxw42
Copy link
Member Author

cxw42 commented Nov 19, 2023

I am able to repro on Vim 9 --- the behaviour is different at e269673^ than at that commit.

VIM - Vi IMproved 9.0 (2022 Jun 28, compiled Nov 19 2023 11:11:34)
Included patches: 1-2100

$ fzf --version
0.24.0-1 (3304f28)

fzf vim plugin 0.44.1

Additionally, I see the same difference at 345a5a3. I think I must have accidentally hit rebase-merge on #209 --- sorry! e269673 is part of the feature branch for #209, and is not in the history of master. 345a5a3 is the relevant commit that is reachable from master.

Edit I wonder if this is similar to junegunn/fzf#2041.

A workaround

junegunn/fzf.vim#1164 (comment) (don't use popup) fixes the problem for me, but at the cost of a change in UX

@cxw42 cxw42 self-assigned this Nov 19, 2023
@cxw42 cxw42 changed the title Issue after #209 Cursor moves back to beginning of line when exiting :FZF popup on Vim 9 (after #209) Nov 19, 2023
@cxw42
Copy link
Member Author

cxw42 commented Nov 19, 2023

@yous @midgleyc It looks like this might be a Vim bug. Would you be willing to try my MCVE at https://github.com/cxw42/vim9-tabstop-repro-2023-11 and see if you observe the same issue? Instructions in the README. Once I hear back from you, I will report on the Vim issue tracker. Thanks!

Summary: It appears setbufvar(&tabstop) changes the cursor position when used in BufFilePost.

Some options include:

  • Revert 345a5a3 (but this might cause old issues related to handling of no-name buffers to resurface)
  • Revert 345a5a3 and remove BufNew --- that is the case for which I made the %/<abuf> change, as far as I can recall.
  • In BufFilePost (or other autocmds?), don't apply settings if we already did so and the buffer's name is unchanged

@cxw42 cxw42 mentioned this issue Nov 19, 2023
@chrisbra
Copy link
Contributor

what is the purpose of the BufFilePost ? It triggers when renaming the terminal buffer of the FZF buffer to a terminal buffer for !sh. The terminal seems to be running in the same window as test.c at least temporarily and so does the setbufvar() option trigger for the terminal window, Vim tries to validate the cursor position of the first line (which is empty) and therefor positions the cursor on the start of the line in that window.

@chrisbra
Copy link
Contributor

I'd suggest to add a condition like this to it to fix it:

function! s:SetTabstop(which) abort
    let l:tabstop = 8
    let l:bufnr = str2nr(expand('<abuf>'))
    " skip this event for terminal buffers
    if bufname(l:bufnr) =~ '^!\w*sh$'
      return
    endif

@yous
Copy link
Contributor

yous commented Nov 20, 2023

I can confirm that observed behaviors mentioned in https://github.com/cxw42/vim9-tabstop-repro-2023-11 are reproducible in macOS.

$ vim --version
VIM - Vi IMproved 9.0 (2022 Jun 28, compiled Nov 11 2023 17:59:33)
macOS version - arm64
Included patches: 1-2100
$ fzf --version
0.44.1 (d7d2ac3)

Also when I added if bufname(l:bufnr) =~ ... condition that @chrisbra mentioned, cursor doesn't move.

@cxw42
Copy link
Member Author

cxw42 commented Nov 24, 2023

@chrisbra @yous thanks very much!

The terminal seems to be running in the same window as test.c at least temporarily

This seems like either a vim or an fzf bug --- @chrisbra do you have any thoughts on which it might be?

Edit Using the repro, when bufnr() of foo.c is 1, I still get:

BufFilePost: l:abuf 2, curr 2; repro 1; bufname !sh

that is, buffer 2 has just been renamed to !sh.

so does the setbufvar() option trigger for the terminal window

but :help tabstop says tabstop is a buffer-local option, not a window-local option.

Oh, I think I see. If I edit an empty file and a nonempty file in a single window, then toggle between them with :e#, cursor-position changes when I move from the nonempty file to the empty file still affect the cursor position when I move back to the nonempty file. In this case, foo.c is the nonempty file and the newly-opened terminal is the empty file. This seems similar to vim/vim#7954 (comment) .

So I think the desired behaviour would be for fzf to open the popup first, and open the terminal in the popup, rather than the other way around. Does that make sense?

Possibly-similar fzf issue: junegunn/fzf.vim#1164

@cxw42
Copy link
Member Author

cxw42 commented Nov 24, 2023

what is the purpose of the BufFilePost?

It was added in aadfe23 "Reload config on file name changes". It looks from :help autocmd.txt like BufFilePost is the only way to catch :file and :saveas, so we need to keep it.

@chrisbra
Copy link
Contributor

chrisbra commented Nov 24, 2023 via email

@cxw42
Copy link
Member Author

cxw42 commented Nov 24, 2023

@chrisbra why does

call setbufvar(l:abuf, '&tabstop', l:tabstop)

behave differently from

" bufnr() == l:abuf
let &l:tabstop = l:tabstop

? (:help eval.txt says that the latter corresponds to :setlocal, so I also tried that, and it does not repro.) I don't understand why "Vim tries to validate the cursor position" when using setbufvar but not when using let &l:/:setlocal. Thanks in advance for any information you can provide!

cxw42 added a commit to cxw42/vim9-tabstop-repro-2023-11 that referenced this issue Nov 24, 2023
cxw42 added a commit to cxw42/vim9-tabstop-repro-2023-11 that referenced this issue Nov 24, 2023
cxw42 added a commit to cxw42/editorconfig-vim that referenced this issue Nov 24, 2023
:FZF opens a terminal in the same window as the current buffer, and
setbufvar(tabstop) affects the cursor position in the current buffer
as well as the cursor position in the terminal buffer.

To avoid that, don't set tabstop in terminal windows running shells.
@chrisbra
Copy link
Contributor

chrisbra commented Nov 24, 2023 via email

cxw42 pushed a commit to cxw42/editorconfig-vim that referenced this issue Nov 24, 2023
:FZF opens a terminal in the same window as the current buffer, and
setbufvar(tabstop) affects the cursor position in the current buffer
as well as the cursor position in the terminal buffer.

To avoid that, don't set tabstop in terminal windows running shells.
@cxw42
Copy link
Member Author

cxw42 commented Nov 24, 2023

@chrisbra thanks for the info!

@yous @midgleyc could you please try the code in #229 and see if it resolves the issue? It works for me, but I would appreciate confirmation before merging. Thank you!

@yous
Copy link
Contributor

yous commented Nov 26, 2023

I don't see the problem with #229. Thanks!

cxw42 pushed a commit that referenced this issue Nov 26, 2023
:FZF opens a terminal in the same window as the current buffer, and
setbufvar(tabstop) affects the cursor position in the current buffer
as well as the cursor position in the terminal buffer.

To avoid that, don't set tabstop in terminal windows running shells.
@yous
Copy link
Contributor

yous commented Nov 28, 2023

I found another problem with #229,

.editorconfig:

root = true

[*]
tab_width = 8

[*.c]
indent_style = space
indent_size = 2

test.c:

#include <stdio.h>

int main() {
}

test.vimrc:

set nocompatible

call plug#begin()
Plug 'editorconfig/editorconfig-vim'
call plug#end()

autocmd FileType c EditorConfigReload
  1. Run vim -u test.vimrc test.c
  2. See set sw
  3. Vim shows shiftwidth=8, not shiftwidth=2

@yous
Copy link
Contributor

yous commented Nov 28, 2023

Ah, l:bufnr should be a:bufnr in bufname(l:bufnr) !~# '^!\w*sh$'.

@yous yous mentioned this issue Dec 1, 2023
@misraelson
Copy link

Just wondering if someone can post the fix here for folks. Currently setting up a new laptop with VIM9 and experiencing this problem. Should I paste this into my .vimrc ?

function! s:SetTabstop(which) abort
    let l:tabstop = 8
    let l:bufnr = str2nr(expand('<abuf>'))
    " skip this event for terminal buffers
    if bufname(l:bufnr) =~ '^!\w*sh$'
      return
    endif

@cxw42
Copy link
Member Author

cxw42 commented Jan 28, 2024

@misraelson If you clone this repo and add it to the beginning of your runtimepath, I think it will take precedence and you will get the fix. I'm not sure, though, as I haven't migrated to vim9 yet. Any vim9 folks know for sure?

I can tell you that pasting s:SetTabstop into your .vimrc won't fix it, since s: means "script", i.e., the current file. You could probably copy the current contents of this repo over the version of the plugin that ships with vim 9, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants