Possible simplification

[g0xA52A2A]

Lifting some of the weight back onto the definition of commands and mappings could allow for simplifications to the function that ultimately gets called, which I think makes things a lot more readable. I came up with the following from essentially trying to write this plugin myself.

function! List(command, pattern) range abort
  let list    = []
  let command = a:firstline . ',' . a:lastline . a:command . ' /' . a:pattern

  try
    let output = split(execute(command), '\n')
  catch
    echomsg v:exception
    return
  endtry

  for line in output
    if filereadable(line)
      let filename = line
    else
      let lnum = split(line)[1]
      let text = split(line, '\v^\s*\d+:\s+\d+\s')[-1]
      call add(list,
        \ { 'filename' : filename
        \ , 'lnum'     : split(line)[1]
        \ , 'col'      : match(text, a:pattern) + 1
        \ , 'text'     : text
        \ })
    endif
  endfor

  call setqflist(list)
endfunction

command! -bang -bar -range=% -nargs=+ Dlist
  \ let b:winview = winsaveview()
  \ | <line1>,<line2> call List('dlist' . <q-bang>, <q-args>)
  \ | call winrestview(b:winview)
  \ | unlet b:winview

command! -bang -bar -range=% -nargs=+ Ilist
  \ let b:winview = winsaveview()
  \ | <line1>,<line2> call List('ilist' . <q-bang>, <q-args>)
  \ | call winrestview(b:winview)
  \ | unlet b:winview

nnoremap <silent> [D     :<C-U>   Dlist <C-R>=expand('<cword>')<CR><CR>
nnoremap <silent> ]D     :<C-U>.,$Dlist <C-R>=expand('<cword>')<CR><CR>

nnoremap <silent> [<C-D> :<C-U>   Dlist<C-R>=v:count ? '!' : ''<CR> <C-R>=expand('<cword>')<CR><Bar>cnext<CR>
nnoremap <silent> ]<C-D> :<C-U>.,$Dlist<C-R>=v:count ? '!' : ''<CR> <C-R>=expand('<cword>')<CR><Bar>cnext<CR>

nnoremap <silent> [I     :<C-U>   Ilist <C-R>=expand('<cword>')<CR><CR>
nnoremap <silent> ]I     :<C-U>.,$Ilist <C-R>=expand('<cword>')<CR><CR>

nnoremap <silent> [<C-I> :<C-U>   Ilist<C-R>=v:count ? '!' : ''<CR> <C-R>=expand('<cword>')<CR><Bar>cnext<CR>
nnoremap <silent> ]<C-I> :<C-U>.,$Ilist<C-R>=v:count ? '!' : ''<CR> <C-R>=expand('<cword>')<CR><Bar>cnext<CR>

Notably I've not done anything with visual mode here which is why I'm not making a pull request at this point. I'd like to clarify a few things before such a step.

From the initial commit this plugin has used the visual selection as the pattern to be used in searches. Whilst this is not the default Vim behavior for the definition and include searches it seems a logical addition. However using the visual selection as the pattern doesn't make as much sense when using jumps like ]_Ctrl-I. This feels a bit disconnected to me. Also from a simplification perspective handling visual mode as is currently done will add some complexity to the above.

In conclusion.

• Is the presented snippet of interest?
• Does visual mode need to be handled as is?
  • If so what if anything should be done for jumps?

[romainl]

Is the presented snippet of interest?

This certainly is more compact but I prefer the complexity to be encapsulated in the function. If you can find a way to simplify the function without changing its interface I will gladly take it.

Does visual mode need to be handled as is?

You are right that visual mode is not handled natively but I feel it is a valuable addition that must stay. As for your remark about ]_Ctrl-I I am not sure I follow: wanting to jump to the first match of an arbitrary selection seems quite legitimate to me. Pressing ]<C-i> on the myObject or the myKey in myObject.myKey might lend you on a different spot than pressing ]<C-i> on the whole myObject.myKey. It may not seem that necessary, but it was very easy to implement with the current interface so it was a cheap addition.

If so what if anything should be done for jumps?

Jumps as in "jumplist"?

[g0xA52A2A]

This certainly is more compact but I prefer the complexity to be encapsulated in the function. If you can find a way to simplify the function without changing its interface I will gladly take it.

Thanks I'll have a think about this.

wanting to jump to the first match of an arbitrary selection seems quite legitimate to me.

Oh I agree. Sorry looking back I didn't make it very clear. Having ]<C-I> use the visual selection as the pattern changes repetition behaviour compared to the default.

If so what if anything should be done for jumps?

Jumps as in "jumplist"?

I meant a jump like ]<C-I>. As it stands this plugin doesn't do anything for ]<C-I> whereas the snippet above does. If this part is of interest, in visual mode what pattern should be used? The visual selection or <cword>?

• The former would be in line with this plugin but is unlikely to be of use repeatedly which makes behaviour quite different from the default.

• The later would be in line with the default behaviour but then makes it less consistent with the rest of the plugin.

Ultimately I guess it comes down to if people use ]<C-I> repeatedly to expand visual selections, which I don't imagine they do. I realise now that this topic should've probably been a separate issue.

[romainl]

Well, the reason I didn't implement the [<C-/]<C- family of commands is that this plugin is all about listing things and those commands only jump to things, without listing them.

But I can very well imagine a visual [<C-i> or ]<C-d>… that would operate on the visual selection, not on <cword>. I don't think it would fit the limited scope of this plugin but it could be a welcome addition to another plugin with a slightly broader "make include search smoother" scope.

Which is, maybe, what you have in mind…

[g0xA52A2A]

Ah the distinction of only dealing with commands that would display a list makes for a clearer separation of concerns.

I'll close this issue and let this snippet live in my config for a bit and see how it works out. If I end up incorporating changes to this plugin I'll open a pull request.