airblade / vim-gitgutter

A Vim plugin which shows git diff markers in the sign column and stages/previews/undoes hunks and partial hunks.
MIT License
8.37k stars 297 forks source link

Undefined variable: b:source_window when staging from a floating preview menu #866

Closed thisisrandy closed 1 year ago

thisisrandy commented 1 year ago

What is the latest commit SHA in your installed vim-gitgutter?

68f16eb

What vim/nvim version are you on?

NVIM v0.9.0

The docs state:

To stage part of any hunk:

  • preview the hunk, e.g. hp;
  • move to the preview window, e.g. :wincmd P;
  • delete the lines you do not want to stage;
  • stage the remaining lines: either write (:w) the window or stage via hs or :GitGutterStageHunk.

Note the above workflow is not possible if you have opted in to preview hunks with Vim's popup windows.

I use all default options except for mappings and have nvim_open_win, so g:gitgutter_preview_win_floating is 1 by default (note that the wording reproduced above is misleading on this point). I have a mapping, reproduced at the bottom of this issue, that finds and focuses on the first floating window, and I use this for selective staging (feel free to direct your users to use something like it in a floating window-based workflow). I don't do selective staging super often, but it's always worked without issue until I tried it today. After writing the window, vim complains:

Error detected while processing BufWriteCmd Autocommands for ""..function gitgutter#hunk#stage[6]..157_hunk_op[25]..157_goto_original_window: line 1: E121: Undefined variable: b:source_window

If I turn g:gitgutter_preview_win_floating off and write the preview window instead, the issue does not occur.

I did some digging, and it looks like the problem is in s:open_hunk_preview_window in autoload/gitgutter/hunk.vim. The checks

  if g:gitgutter_preview_win_floating
    if exists('*nvim_open_win')

both pass for me, so the function returns at line 475. However, source_window, which is set at the top of the function, isn't assigned to b:source_window until further down at line 509. This is a relatively recent change (11d6e131), which explains why I've never encountered it before (as I said, I don't use this feature very often).

I tried just doing away with source_window, which is only ever used once to assign its value to b:source_window, and instead assigning directly to b:source_window at the top of the function. This of course fails, because the variable is buffer-scoped and therefore not defined in the floating window. However, it works fine if we make it ~global~ a script variable, so I'm going to submit a PR with that change. Let me know what you think.


My focus function for reference:

" Focus on the first floating window
function! s:GotoFirstFloat() abort
  for w in range(1, winnr('$'))
    let c = nvim_win_get_config(win_getid(w))
    " c.relative isn't set for normal windows. see nvim_win_get_config()
    if c.focusable && !empty(c.relative)
      execute w . 'wincmd w'
      break
    endif
  endfor
endfunction
noremap <silent> <c-w><space> :<c-u>call <sid>GotoFirstFloat()<cr>
airblade commented 1 year ago

Thanks for reporting this. (I didn't notice it because I use Vim and non-floating previews.)

Thanks also for the analysis. However b:source_window is only needed when using non-floating preview windows.

I have fixed it in a7a83c3.

thisisrandy commented 1 year ago

Well-observed. Thanks.