-
Notifications
You must be signed in to change notification settings - Fork 14
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
WIP: Suggested improvements #49
base: vim9
Are you sure you want to change the base?
Conversation
97461f4
to
d691460
Compare
Hi @Donaldttt, First, thank you for Fuzzyy, it's a great plugin, I love the aim to have something that just works out of the box, that's exactly what I was looking for after using fzf.vim for years and leaderf for a while recently. I think it's time I asked for your feedback on some of these suggested changes before going much further. They are mostly either minor bug fixes or changes aiming to improve the out of the box experience so it "just works" as expected for new users (as expected being obviously highly subjective). They are driven by my personal use over a couple of weeks and my thoughts on how this should work as a plugin I would recommend to relative newcomers to Vim (I have recommended it btw!). If you have a little time could you take a look at let me know what you think. I don't expect you to agree with every change, but I hope I have explained each one adequately and you can understand why I've made it, if not, please let me know and I'll clarify. Thanks! |
Unnecessary as --vimgrep option ensures column numbers are included
5649c9c
to
2c30337
Compare
Yes, a breaking and contentious change, but also one I think most users would find the least surprising default (LeaderF and Telescope.nvim both support fd by default but also respect .gitignore by default). This will disrupt existing users, but I still believe it's a worthwhile change.
Using --full-name outputs paths relative to the repository root, but the other find commands output paths relative to the current directory. Before this change, when git-ls-files was used in a repo subdirectory, opening files from the results would not work as paths would not exist.
This behaves the same, but is, I think, easier to read and understand
Use fixed strings rather than regular expressions when searching with grep program, consistent with options used for ag, rg, and findstr
fuzzyy options should really be prefixed with fuzzyy_
When in a git repo use git-grep when no ag or rg, include untracked but respect gitignore. Similar to use of git-ls-files with FuzzyFiles.
For findstr, best we can do is ignore files with unprintable characters. Note that I've replaced /i with /P for findstr, I didn't realise findstr parameters were case-insensitive so I previously added /i, not realising that /I was the same thing, and the search was already case-insensitive.
These options are used when gitignore is not, or cannot, be respected, and in the case of the preferred files command, fd, sets -E/--exclude options, which "overrides any other ignore logic" (see man fd) The naming of these variables, using ignore rather than exclude, may be confusing in this context, and possibly even more confusing if future variables closely align with fd and rg options like --no-ignore and --no-ignore-parent (as per telescope.nvim, e.g. no_ignore_parent)
CurSearch typically has solid background, which leads to a very hard to read wall of solid color, often orange or yellow, when fuzzy matching. I think this default should be replaced with something easier to read. Telescope.nvim uses Special by default, so use Special. Personally I prefer QuickFixLine as it's more consistent among color schemes, but it's probably better to copy a convention from a very popular plugin.
Trying to restore cursor by saving and resetting &guicursor doesn't work in MacVim r180 (Vim 9.1.727), when &guicursor starts at n:blinkon0 and is restored after closing the popups, the cursor is no longer visible. Fix by clearing the Cursor highlight group and restoring it instead. This is similar to what LeaderF does (adds and then deletes a match). Note that capturing the before state now happens when creating the popup windows, not when loading popup.vim, to allow for color scheme changes. P.S. Also tested that this works with hi-link, and with gVim on Windows.
Only execute the command if it has no arguments. If the command has arguments, even optional arguments, place the command in the command line on selection, followed by a single space (same as telescope.nvim) Note: also tested with gVim on Windows
Avoid error executing another Fuzzy command from FuzzyCommands results Error detected while processing function <SNR>148_MenuFilter: line 1: E716: Key not present in Dictionary: "1004" Error detected while processing function <SNR>148_GeneralPopupCallback: line 23: E716: Key not present in Dictionary: "1004" Note: unrelated to my other changes, the bug is present on vim9 branch
Enable err_cb for async jobs to run ag, rg, rd etc., if things go wrong, users should be able to see that something has gone wrong (will be more important when greater customisation of commands or options is allowed).
Do not include patterns specific to particular operating systems, build tools, programming language etc., it's almost impossible to set defaults including these that are widely applicable and stand the test of time. The new defaults only include files known to Vim, i.e. swap files and the default tags file, and the most common version control directories. It should now be reasonable to always apply these exclusions, as I think the original author of the feature did, but will add a separate commit.
This makes sense to me, the defaults are now set to values that you probably always want to ignore, vim swap files, tags files, common version control directories, and users may want to always exclude some files or directories without adding to gitignore or other ignore files. For example, I might like to add tags.* to this, for Vim gutentags. Sure, I could add to a "global" ignore file, but that won't necessarily work with --no-ignore-parent, and maybe I want --no-ignore-parent. Also, fd and rg don't follow include directives in git config, so if you set a "global" gitignore using core.excludesFile from a config file included from your global gitconfig it doesn't work as expected with fd and rg.
- fuzzyy options should really be prefixed with fuzzyy_ - use "mappings" to avoid confusion with fuzzy_keymaps
No longer necessary, __vista__ buffers are unlisted anyway, and FuzzyBuffers hides unlisted buffers. Also intentionally removed the setting of a default value in plugin/fuzzyy.vim to be consistent with how other options specific to particular autoload files are set.
510ad03
to
d1b4684
Compare
Use :highlight command to create custom highlight groups for fuzzyy highlighting, all linking to existing highlight groups by default. There are no changes to the actual highlighting here, this just changes how the matched highlight can be customised by the user, and allows other highlight groups to be customised, rather than being hard-coded.
d1b4684
to
c01a0f9
Compare
Hi, thank you for all the work you have done. I have been very busy lately. I will take a look of these change soon. |
c4af10a
to
a1c6c34
Compare
Automatically include ignore case option when there are no upper case characters in pattern to search. Mimics ag and rg --smart-case option.
As the command strings for files and grep are computed at load time, the command string is effectively cached, never computed again at runtime. If the FuzzyyFiles or FuzzyyGrep commands are used first within a git repo and the derived command is a git command, git-grep or git-ls-files, that command will be used again after the user the changes the working directory to somewhere that is not within a git repo, and won't work. The same issue applies in reverse, user starts off in a non-git repo directory, then changes to git repo, fallback git commands not used. The fix for FuzzyyFiles is nice and neat, but fix for FuzzyyGrep is a lot messier as a number of things change when the command changes. I'm sure this can be improved, but better to fix first and improve later.
a1c6c34
to
23a922b
Compare
TODO:
g:fuzzyy_mru_project_only
with:FuzzyyMruCwd
command (referring to this as project only is a bit misleading anyway, current working directory is not necessarily project root, but:FuzzyyMruProject
would be nice too):FuzzyyMRUFiles
toFuzzyyMru
because shorter,Files
is obvious (I think), and:FuzzyyMRUCWD
would be horrid