search

editorconfig / editorconfig-vim

EditorConfig plugin for Vim
http://editorconfig.org
Other
3.11k stars 135 forks source link

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

Closed cxw42 closed 7 months ago

cxw42 commented 11 months ago

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:

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 https://github.com/editorconfig/editorconfig-vim/issues/209#issuecomment-1680605824

cxw42 commented 11 months ago

@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 commented 11 months ago

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 commented 11 months ago

Thanks for the details!

midgleyc commented 8 months ago

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 commented 8 months ago

@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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

cxw42 commented 8 months ago

I am able to repro on Vim 9 --- the behaviour is different at e26967348^ 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 345a5a32a808. I think I must have accidentally hit rebase-merge on #209 --- sorry! e26967348 is part of the feature branch for #209, and is not in the history of master. 345a5a32a808 is the relevant commit that is reachable from master.

Edit I wonder if this is similar to https://github.com/junegunn/fzf/issues/2041.

A workaround

https://github.com/junegunn/fzf.vim/issues/1164#issuecomment-756855784 (don't use popup) fixes the problem for me, but at the cost of a change in UX

cxw42 commented 8 months ago

@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:

chrisbra commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

@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 https://github.com/vim/vim/issues/7954#issuecomment-797704153 .

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: https://github.com/junegunn/fzf.vim/issues/1164

cxw42 commented 7 months ago

what is the purpose of the BufFilePost?

It was added in aadfe238e3beb342c80e46eb2e928c7141a762c3 "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 commented 7 months ago

Not sure, It may be the normal way how the terminal is started inside the popup. I would rather add the condition to skip the auto command for buffer names starting with ! (indicating a terminal). Or perhaps that auto command should use and only trigger for buffers where editorconfig is actually active, instead of for all buffers.

Am 24. November 2023, um 20:37, schrieb Chris White @.***>:

@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?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

[ { @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "https://github.com/editorconfig/editorconfig-vim/issues/224#issuecomment-1826033736", "url": "https://github.com/editorconfig/editorconfig-vim/issues/224#issuecomment-1826033736", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

cxw42 commented 7 months ago

@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!

chrisbra commented 7 months ago

One is switching buffers, before setting the option value, the other is just setting the buffer local variable for the current buffer.

Am 24. November 2023, um 21:06, schrieb Chris White @.***>:

@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!

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

[ { @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "https://github.com/editorconfig/editorconfig-vim/issues/224#issuecomment-1826051347", "url": "https://github.com/editorconfig/editorconfig-vim/issues/224#issuecomment-1826051347", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

cxw42 commented 7 months ago

@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 commented 7 months ago

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

yous commented 7 months ago

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 commented 7 months ago

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

misraelson commented 6 months ago

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 commented 5 months ago

@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.