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

Improve file/path completion, handle more cases #256

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

mmrwoods
Copy link

@mmrwoods mmrwoods commented Apr 20, 2022

Hi,

First, thanks for vim-grepper, this is a great plugin.

The prompt is brilliant, and neatly fixes the problem of having to manually escape # and friends when searching. One thing that immediately threw me though was relative path completion, I typed a search pattern, followed by the first word character in a directory within which I wanted to search, and got no completion :-(

So, I looked at the code and made some changes, supported by some new test coverage, to address my new requirement.

As I'd already added some test coverage, I then went on to add a few more features, e.g. tilde expansion, fix completion of hidden files in cwd, and made some minor changes to the completions returned so it behaves a little more like vim's built in completion, e.g. complete f with foo/, not ./foo/.

Seems to be working well, though I have not battle tested these changes. Would be interested to get some feedback. Do you think these changes are even sensible?

@mmrwoods mmrwoods force-pushed the improve-file-completion branch 2 times, most recently from f1b96e1 to a662b81 Compare April 23, 2022 08:47
I want to improve file/path completion, so add some tests to cover
existing behaviour (at least existing behaviour as I understand it).
Relative path completion relies on the globpath() function, prepending
a leading dot and slash when creating the path for globpath(), but this
results in path completions with a leading dot and slash, unlike vim's
built in file/path completion, so strip them back out before returning.
- Expand tilde alone to $HOME (same as vim's built in path completion)
- Support completion of paths relative to ~/ (taken from PR mhinz#152)

This is getting quite messy now, but that's ok, we have some tests!
Refactoring can come later, maybe when supporting hidden files/dirs.
@mmrwoods mmrwoods changed the title WIP: Improve file/path completion Improve file/path completion, handle more cases May 6, 2022
@mmrwoods
Copy link
Author

mmrwoods commented May 6, 2022

@mhinz I've reworked these commits after I found some time to RTFM :-)

I think this PR is in a good state now, if you'd like to take a look.

I did also consider changing the default completion for the Grepper command to list all flags, but I wasn't sure about that (maybe you should have to add a dash to start completing on flags).

FWIW, here is a patch that would add it (note the hack to deal with abandoned completions):

diff --git a/plugin/grepper.vim b/plugin/grepper.vim
index 199bbb7..689c863 100644
--- a/plugin/grepper.vim
+++ b/plugin/grepper.vim
@@ -212,12 +212,12 @@ endfunction
 " Completion {{{1
 " grepper#complete() {{{2
 function! grepper#complete(lead, line, _pos) abort
+  let flags = ['-append', '-buffer', '-buffers', '-cd', '-cword', '-dir',
+        \ '-grepprg', '-highlight', '-jump', '-open', '-prompt', '-query',
+        \ '-quickfix', '-side', '-stop', '-switch', '-tool', '-noappend',
+        \ '-nohighlight', '-nojump', '-noopen', '-noprompt', '-noquickfix',
+        \ '-noside', '-noswitch']
   if a:lead =~ '^-'
-    let flags = ['-append', '-buffer', '-buffers', '-cd', '-cword', '-dir',
-          \ '-grepprg', '-highlight', '-jump', '-open', '-prompt', '-query',
-          \ '-quickfix', '-side', '-stop', '-switch', '-tool', '-noappend',
-          \ '-nohighlight', '-nojump', '-noopen', '-noprompt', '-noquickfix',
-          \ '-noside', '-noswitch']
     return filter(map(flags, 'v:val." "'), 'v:val[:strlen(a:lead)-1] ==# a:lead')
   elseif a:line =~# '-dir \w*$'
     return filter(map(['cwd', 'file', 'filecwd', 'repo'], 'v:val." "'),
@@ -228,7 +228,7 @@ function! grepper#complete(lead, line, _pos) abort
     return filter(map(sort(copy(g:grepper.tools)), 'v:val." "'),
           \ 'empty(a:lead) || v:val[:strlen(a:lead)-1] ==# a:lead')
   else
-    return []
+    return flags
   endif
 endfunction
 
@@ -645,6 +645,8 @@ function! s:parse_flags(args) abort
       endif
       let flags.cd = dir
       break
+    elseif flag ==# '-'
+      redraw!
     else
       call s:error('Ignore unknown flag: '. flag)
     endif

This is the default case, it doesn't need an explicit condition
Don't need to differentiate any more as we are stripping out the leading
dot and slash from the resulting glob matches. Tests still pass :-)
As far as I can tell, this does not make sense for the Grepper command.
The command opens the prompt, from which path completion is performed.

I suspect the right thing to do here is complete with all flags, but it
seems sensible to leave that for another commit to make changes clear.
e.g. complete ~ with /home/foo/ rather than /home/foo, as vim does
Remove custom handling of paths relative to ~/, unnecessary
File completion is now delegated to Vim's built in getcompletion()
function, and the tests for grepper#complete arguably unnecessary.
@mmrwoods
Copy link
Author

@mhinz This has now been simplified to just use Vim's built in getcompletion() function for file completions.

tylerw added a commit to tylerw/vim-grepper that referenced this pull request Sep 7, 2022
tylerw added a commit to tylerw/vim-grepper that referenced this pull request Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant