junegunn / fzf.vim

fzf :heart: vim
MIT License
9.52k stars 582 forks source link

Remove dependencies of :Helptags on grep and sort to make it easier to get it working on Windows #1482

Closed edwinst closed 1 year ago

edwinst commented 1 year ago

This removes fzf#vim#helptags dependencies on external "grep" and "sort" commands. I think it also makes the code simpler. The special filename handling for Windows could be removed.

I put the removal of the external "sort" in a separate commit since it can affect the sort order. E.g. my Windows "sort" command puts single-quote (') before bang (!), while Perl's sort does the reverse. (Perl sorts these in ASCII order.)

Note that this does not make :Helptags fully work for me on Windows. There is one further problem: preview.sh is called with colons as separators for the filenames, etc., which breaks on Windows. I fixed it locally but I'm not sure what a good universal separating character other than colon could be. I used '$' locally but that may not be a good choice for Linux users.

junegunn commented 1 year ago

Thanks.

But I'm getting different results with and without the patch

without patch

image

with patch

image
edwinst commented 1 year ago

That's very strange. You seem to get almost 10x more results with the patch and still the lines starting with bang '!' are missing, although they should be the first ones.

I will try to repro that in Linux. Sorry about that.

edwinst commented 1 year ago

On Linux I get different sort order but the same number of results. I created a new pull request that only replaces "grep" and keeps the external "sort", in order to keep the sort order. (Windows sort is pretty useless but Linux sort seems to give dictionary order by default which is kind of useful.)

https://github.com/junegunn/fzf.vim/pull/1483

This should at least help to narrow down the problem. I still don't know why you get a different number of results with the patch, unfortunately. Maybe you have a minute to test the new pull request. Thanks!

edwinst commented 1 year ago

BTW, if you can spare the time and the new pull request still gives you wrong results, please consider inserting a "| tee /tmp/debug.txt" into the pipe before the sort and gist the results before/after the patch so I can have a look at what's going on.

junegunn commented 1 year ago

There was a bug in your Perl script. No duplicates now.

edwinst commented 1 year ago

D'oh, that was a stupid bug! I probably missed it because on my system the only large file came last, so I did not notice the duplications.

There's still the user-facing question of what is the preferred sort order. GNU sort seems to do some kind of dictionary order by default which might be perceived as more useful than the codepoint order produced by Perl's sort:

$ (echo '!test' ; echo bar) | sort
bar
!test
$ (echo '!test' ; echo bar) | perl -ne 'push @a,$_; END { print for sort @a }'
!test
bar

That's why I would have leaned towards #1483 , in order not to change possibly desired behavior for Linux users.

For Windows, the only other thing that keeps :Helptags from working is that the tagpreview.sh script expects some arguments, including the filename, to be separated by colons, which breaks on Windows. I'll try to find a robust fix for that, if my time permits.

junegunn commented 1 year ago

The results are identical on my system, so I think we're good to go.

$ # macOS sort
$ (echo '!test' ; echo bar) | sort
!test
bar

$ # GNU sort
$ (echo '!test' ; echo bar) | gsort
!test
bar

$ (echo '!test' ; echo bar) | perl -ne 'push @a,$_; END { print for sort @a }'
!test
bar
junegunn commented 1 year ago

Merged, thanks!