Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Don't match filepath in grep2 #385

Merged
merged 13 commits into from
Apr 10, 2020
Merged

Don't match filepath in grep2 #385

merged 13 commits into from
Apr 10, 2020

Conversation

liuchengxu
Copy link
Owner

@liuchengxu liuchengxu commented Apr 9, 2020

Fix #383 (comment)

Also fix some errors found in maple.

@vn-ki
Copy link

vn-ki commented Apr 9, 2020

Yes, this work!

I found more problems with grep2 though.

image
image

As you can see fuzzy is given more preference in grep2 than letter case. I find this behaviour very weird.

@vn-ki
Copy link

vn-ki commented Apr 9, 2020

See how the result changes when one extra letter is added,

image
image

@liuchengxu
Copy link
Owner Author

liuchengxu commented Apr 9, 2020

Just like what I have mentioned in https://github.com/liuchengxu/vim-clap/blob/master/CHANGELOG.md#added, that's expected at the moment, for grep2 is forced to use the fzy fuzzy algorithm, but grep is not, which returns the result of ripgrep directly. We could borrow the extended search syntax of grep to grep2 later, prepend ' at the beginning of the query to not use the fuzzy filtering. But for now, that's how grep2 works.

@liuchengxu
Copy link
Owner Author

@vn-ki Can you show me the link of the repo used in your above picture?

@vn-ki
Copy link

vn-ki commented Apr 9, 2020

https://github.com/vn-ki/harsark.rs

I find fuzzy really unsuitable for text searching.

image
image

@liuchengxu
Copy link
Owner Author

liuchengxu commented Apr 9, 2020

@vn-ki You just spot a bug of clap! This patch should help at present.

diff --git a/autoload/clap/filter/async/dyn.vim b/autoload/clap/filter/async/dyn.vim
index 66b043b..6e3e732 100644
--- a/autoload/clap/filter/async/dyn.vim
+++ b/autoload/clap/filter/async/dyn.vim
@@ -30,7 +30,7 @@ function! clap#filter#async#dyn#from_tempfile(tempfile) abort
   else
     let enable_icon_opt = ''
   endif
-  let filter_cmd = printf('%s --number 100 --winwidth %d filter "%s" --input "%s"',
+  let filter_cmd = printf('%s --number 30 --winwidth %d filter "%s" --input "%s"',
         \ enable_icon_opt,
         \ winwidth(g:clap.display.winid),
         \ g:clap.input.get(),
@@ -41,7 +41,7 @@ endfunction
 
 function! clap#filter#async#dyn#start_grep() abort
   let s:last_query = g:clap.input.get()
-  let grep_cmd = printf('%s --number 100 --winwidth %d grep "" "%s" --cmd-dir "%s"',
+  let grep_cmd = printf('%s --number 30 --winwidth %d grep "" "%s" --cmd-dir "%s"',
         \ g:clap_enable_icon ? '--enable-icon' : '',
         \ winwidth(g:clap.display.winid),
         \ g:clap.input.get(),
@@ -53,7 +53,7 @@ endfunction
 
 function! clap#filter#async#dyn#grep_from_cache(tempfile) abort
   let s:last_query = g:clap.input.get()
-  let grep_cmd = printf('%s %s --number 100 --winwidth %d grep "" "%s" --input "%s"',
+  let grep_cmd = printf('%s %s --number 30 --winwidth %d grep "" "%s" --input "%s"',
         \ g:clap_enable_icon ? '--enable-icon' : '',
         \ has_key(g:clap.context, 'no-cache') ? '--no-cache' : '',
         \ winwidth(g:clap.display.winid),

截屏2020-04-10 上午12 46 36

The problem is this number 30 is not configurable for this reason:

const ITEMS_TO_SHOW: usize = 30;

To make clap faster, I force to only take top 30 items in the results. People with a big screen might think 30 rows are not enough, but we should know that the performance can be worse when we want to show more top items. Maybe we could allow it to have a few limited options, e.g., [30, 40, 50].

@liuchengxu liuchengxu merged commit 6f35788 into master Apr 10, 2020
@delete-merged-branch delete-merged-branch bot deleted the do-not-match-grep-filepath branch April 10, 2020 12:57
@liuchengxu
Copy link
Owner Author

@vn-ki The issue you found should be fixed on mater now. The future improvements could be to add new filter approach aside from the current fuzzy filter ways, e.g, plain string match. https://github.com/junegunn/fzf#search-syntax is also nice to have, but it might need quite a work.

@liuchengxu
Copy link
Owner Author

@vn-ki Another dyn filter bug has been fixed, now the filtered results should be right e53d3c1 , I was wondering why some entries are not put on the top.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants