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

Git filename completion only work with starting pattern #5476

Closed
WonderCsabo opened this issue Jan 3, 2019 · 20 comments
Closed

Git filename completion only work with starting pattern #5476

WonderCsabo opened this issue Jan 3, 2019 · 20 comments

Comments

@WonderCsabo
Copy link

@WonderCsabo WonderCsabo commented Jan 3, 2019

For example, when i am executing git add, and i have this file:

src/SomeClass.java

The completion only work if i start to write src. In the Fish 2.7.1, i could start writing Some and it automatically expanded it to the full file name.

I am using Fish 3.0.0, iTerm2 3.2.6, MacOS Mojave.

I can reproduce it when i disable 3rd party customisations.

@faho
Copy link
Member

@faho faho commented Jan 3, 2019

This works for me.

Can you post the output of the following script, run inside the git repository:

for f in $fish_complete_path/git.fish
    test -f $f; and echo $f
end

command git --version
echo $version

complete -C'git add '

@WonderCsabo
Copy link
Author

@WonderCsabo WonderCsabo commented Jan 4, 2019

Here is the output of the script:

/usr/local/Cellar/fish/3.0.0/share/fish/completions/git.fish
git version 2.20.1
3.0.0
app/build.gradle        Modified file
:/app/build.gradle      Modified file
app/    Directory
app/ci.gradle   Modified file
:/app/ci.gradle Modified file
app/    Directory
git.fish        Untracked file
:/git.fish      Untracked file

When i typing build, the autocomplete does not work.

@faho
Copy link
Member

@faho faho commented Jan 4, 2019

Okay, the script shows it working. It's completing files even though the token is empty.

So: bind \t?

@WonderCsabo
Copy link
Author

@WonderCsabo WonderCsabo commented Jan 4, 2019

I executed bind \t.

bind --preset \t complete

It is still not working. Please note if i start writing app and then hit TAB, it works well. It also completes git commands, like add, with TAB.

@faho
Copy link
Member

@faho faho commented Jan 4, 2019

git add <TAB> should be completing things like "app/build.gradle". Does that happen for you?

@WonderCsabo
Copy link
Author

@WonderCsabo WonderCsabo commented Jan 4, 2019

Yes it does.

 git add <TAB>
:/app/build.gradle  (Modified file)  :/git.fish  (Untracked file)  app/build.gradle  (Modified file)  git.fish  (Untracked file)
:/app/ci.gradle     (Modified file)  app/             (Directory)  app/ci.gradle     (Modified file)

Although as you can see, i am seeing some duplicates starting with :, which is strange.

But if i write

git add build <TAB>

Nothing is suggested for completion.

@faho
Copy link
Member

@faho faho commented Jan 4, 2019

i am seeing some duplicates starting with :, which is strange.

In git, :/ refers to the root of the repo. So wherever you are in the repo, :/app/build.gradle will always refer to the same file as "app/build.gradle" currently.

But if i write

git add build <TAB>

Nothing is suggested for completion.

Ah, I think now I get what you mean! It's not fuzzy-matching inside directories anymore.

@WonderCsabo
Copy link
Author

@WonderCsabo WonderCsabo commented Jan 4, 2019

In Fish 2.7.1

  • i think it did not add duplicates with :.

  • fuzzy-matching was really useful! :( What is the reason for removal?

@faho
Copy link
Member

@faho faho commented Jan 4, 2019

i think it did not add duplicates with :.

Those are on purpose. It's so you can refer to any file from any place in the repo.

@faho
Copy link
Member

@faho faho commented Jan 4, 2019

fuzzy-matching was really useful! :( What is the reason for removal?

Performance. In bigger repos, matching all files is really slow.

I'm checking if we can bring it back, possibly in a limited fashion, but I think I'm actually hitting a git bug - git status -- '*e*' matches files in the repo with "e" in the name, but git --glob-pathspecs status -- '*e*' doesn't. We want to explicitly enable glob pathspecs though, because people might set other options, and that'd break the completions. But glob pathspecs are supposed to be the default, so those calls should be entirely equivalent?

@faho
Copy link
Member

@faho faho commented Jan 4, 2019

I'm checking if we can bring it back, possibly in a limited fashion

No, that would still be too slow.

Sorry, this is currently the best compromise, until #23 is fixed.

@faho faho closed this Jan 4, 2019
@WonderCsabo
Copy link
Author

@WonderCsabo WonderCsabo commented Jan 4, 2019

I see, thanks. :(

For the files starting with :, maybe if git is executed from the root of the repo, those can be omitted, since they are indeed duplicates in that case. I think people usually execute git commands from the repo root.

@WonderCsabo
Copy link
Author

@WonderCsabo WonderCsabo commented Jan 8, 2019

Moreover, can we put fuzzy matching behind a flag, so people can turn it on? Personally i never encountered performance issues, despite working with larger repositories.

@javamuc
Copy link

@javamuc javamuc commented Jan 8, 2019

For anybody who wants this behavior back:

The probably not ideal solution I'm using is copying the git.fish file from 2.7 to ~/.config/fish/completions/git.fish and removing /usr/local/Cellar/fish/3.0.0/share/fish/completions/git.fish

Don't know on what new goodies I'm missing out with this but here we are.

@faho
Copy link
Member

@faho faho commented Jan 8, 2019

and removing /usr/local/Cellar/fish/3.0.0/share/fish/completions/git.fish

@javamuc: That's unnecessary. ~/.config/fish/completions is before the /usr stuff in $fish_complete_path, so if a git.fish exists there it'll be used instead of the other one.

Now, if you want to use the new completions, but complete even fuzzy in subdirectories, take the new 3.0 git.fish, copy it to ~/.config/fish/completions/git.fish, and remove these lines:

    # Glob just the current token for performance
    # and so git shows untracked files (even in untracked dirs) for that.
    # If the current token is empty, this matches everything in $PWD.
    set -l files (commandline -ct)
    # The trailing "**" is necessary to match files inside the given directories.
    set files "*$files*" "$files*/**"

@faho faho reopened this Jan 8, 2019
@javamuc
Copy link

@javamuc javamuc commented Jan 8, 2019

@faho thanks for the pointers. I did what you suggested, but it is still not as expected.

There is only one file that matches what I am typing but it gives me two options:

:/src/main/resources/application.properties  (Modified file)  
src/main/resources/application.properties  (Modified file)

Can you please advise on that too?

@JoshNavi
Copy link

@JoshNavi JoshNavi commented Jan 11, 2019

@javamuc I was having a similar issue, I commented out everything from the if set -q file[1] block in __fish_git_files except for

    set file (string trim -c \" -- $file)
    printf '%s\t%s\n' "$file" $desc

here's the full block:

if set -q file[1]
    # Without "-z", git sometimes _quotes_ filenames.
    # It adds quotes around it _and_ escapes the character.
    # e.g. `"a\\b"`.
    # We just remove the quotes and hope it works out.
    # If this contains newlines or tabs,
    # there is nothing we can do, but that's a general issue with scripted completions.
    set file (string trim -c \" -- $file)
    # First the relative filename.
    printf '%s\t%s\n' "$file" $desc
    # # Now from repo root.
    # set -l fromroot (builtin realpath -- $file 2>/dev/null)
    # and set fromroot (string replace -- "$root/" ":/" "$fromroot")
    # and printf '%s\t%s\n' "$fromroot" $desc

    # And the containing directory.
    # TODO: We may want to offer the parent, but only if another child of that also has a change.
    # E.g:
    # - a/b/c is added
    # - a/d/e is modified
    # - a/ should be offered, but only a/b/ and a/d/ are.
    #
    # Always offering all parents is overkill however, which is why we don't currently do it.
    # set -l dir (string replace -rf '/[^/]+$' '/' -- $file)
    # and printf '%s\t%s\n' $dir "$dir_desc"
end

hope this helps you!

@faho faho mentioned this issue Jan 12, 2019
7 tasks
@faho faho closed this in 73bae38 Jan 13, 2019
@zanchey zanchey added this to the fish 3.1.0 milestone Jan 14, 2019
@zanchey zanchey removed this from the fish 3.1.0 milestone Jan 18, 2019
@zanchey zanchey added this to the fish 3.0.1 milestone Jan 18, 2019
zanchey added a commit that referenced this issue Jan 21, 2019
This enables fuzzy-matching outside of the current directory again.

As it turns out, the performance impact here isn't as large as I
thought - it's massively dependent on caching.

Fixes #5476.

(cherry picked from commit 73bae38)
@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Jan 22, 2019

Reopening for 3.0.1. @faho can you please pick the right thing here into Integration_3.0.1? This looks like it was a complicated one.

@zanchey
Copy link
Member

@zanchey zanchey commented Jan 22, 2019

I took 73bae38 and cherry-picked it as 7a163e8 into Integration_3.0.1, but perhaps @faho can confirm that is all that is needed.

@faho
Copy link
Member

@faho faho commented Jan 22, 2019

6d11e46 is a fixup for a teensy corner-case.

I'm still cherry-picking it and 787f453, another small fix (#5412).

@faho faho closed this Jan 22, 2019
abekoh added a commit to abekoh/dotfiles that referenced this issue Feb 7, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants