lambdalisue / vim-gina

👣 Asynchronously control git repositories in Neovim/Vim 8
http://www.vim.org/scripts/script.php?script_id=5531
MIT License
687 stars 27 forks source link

Unreachable code in autocomplete for commands #260

Closed ptn closed 3 years ago

ptn commented 4 years ago

This conditional in command.vim has a return in each of its branches, which means that all of the code after it never runs:

function! gina#command#complete(arglead, cmdline, cursorpos) abort
  if a:cmdline =~# '^.\{-}Gina!'
    return gina#command#complete(
          \ a:arglead,
          \ substitute(a:cmdline, '^\(.\{-}\)Gina!', '\1Gina _raw', ''),
          \ a:cursorpos,
          \)
  elseif a:cmdline =~# printf('^.\{-}Gina\s\+%s$', a:arglead)
    return gina#complete#common#command(a:arglead, a:cmdline, a:cursorpos)
  else
    return []
  endif
  " ...unreachable code...

git blame tells me that the intention was to not do autocomplete for non-Gina commands. I believe the code should look something like this: (not meant for merging, just for explanation): https://github.com/ptn/gina.vim/commit/288abe535b7c862bf64c8e029baedf579d8ba454

  1. If you type Gina checkout <TAB> with command.vim as it is in master, there is no autocompletion.
  2. If you take the version of command.vim in my commit, then Gina checkout <TAB> does autocomplete.
  3. In neither case is there autocompletion for non-gina commands.

The unreachable code is almost the same as the first branch in the if anyway, though I'm not sure about that [:g:gina#complete_threshold], which the if branch doesn't have.

lambdalisue commented 4 years ago

I don't remember the detail but it's a kind of leftover of debugging code. Could you send me a PR so that I can check it in this weekend?

ptn commented 4 years ago

Sure. For context, this was the issue that introduced that code https://github.com/lambdalisue/gina.vim/issues/204.

Should I add gina#complete_threshold to the topmost return in that function? My PR would delete all of this code, and since the first branch of the conditional does not use that variable, then it would be lost.

lambdalisue commented 4 years ago

It's ok. I'd like to see a PR first 👍