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 diff: filter file completion to paths containing changes #2115

Closed
mytskine opened this issue Jan 4, 2024 · 5 comments · Fixed by #2116
Closed

git diff: filter file completion to paths containing changes #2115

mytskine opened this issue Jan 4, 2024 · 5 comments · Fixed by #2116
Labels
enhancement New feature or request

Comments

@mytskine
Copy link

mytskine commented Jan 4, 2024

Request

git diff -- <TAB> will suggest all the files in the current directory. Other completion systems filter this list so that only relevant files appear in the suggestions.
Same for git diff ./<TAB>.
Would also be great if git diff mydirec<TAB> would work like this.

Proposed solution

I have a branch where git diff -- works as I'm used to, i.e. completes one level of path at a time, ignoring the unchanged files and directories.
master...mytskine:carapace-bin:git-diff-cached

It still lacks path normalization. For instance, having a prefix like ./ breaks completion.

Anything else?

I tried to apply this the other file completion of git-diff, but the prefix makes it hard. I think it would require converting the partial value into a relative path, i.e. "./a" into "a" and "$(cwd)/a" into "a".

But I think it shows a bigger problem with carapace's completion of paths in git commands. I find it inconsistent, and that's the only thing preventing me from using this excellent tool. For instance, with a modified file inside the directory completers/ :

git add com<TAB> (success, smart completion, by file list)
git add ./com<TAB> (fails)
git add -- com<TAB> (fails)
git add -- ./com<TAB> (fails)
git diff ./com<TAB> (success, dumb completion, by path)
git diff com<TAB> (fails)
git diff -- com<TAB> (success, dumb completion, by file list) (my branch: smart completion, by path)
git diff -- ./com<TAB> (fails)
git log com<TAB> (fails)
git log ./com<TAB> (success, by path)
git status (success for all 4 cases, by path)

(edit: two use cases were swapped)

@mytskine mytskine added the enhancement New feature or request label Jan 4, 2024
@rsteube
Copy link
Member

rsteube commented Jan 4, 2024

The git completer was build over years, so it's no surprise there are some inconsistencies.
Commands in git are heavily overloaded and I'm constantly learning how they can be used.
Some features in carapace also weren't available before.

I'm all for improving consistency and only completing changed files when others aren't appropriate.
I think I've already added it to commands like git add and git checkout.

Still quite fond of the ./ prefix in commands like git log though 🤔 .
Files get mixed with the refs (only zsh groups by tags) and everything gets a bit chaotic.
Powershell for example still has no pagination which makes this worse.

@rsteube
Copy link
Member

rsteube commented Jan 4, 2024

I've been meaning to add a config option for a while so things like this could be set to personal preferences.

@mytskine
Copy link
Author

mytskine commented Jan 4, 2024

Thank you for your comments. I know Git has a huge surface that evolves with each release, and I'm uneasy because I'm not always sure of how I'd want Carapace to deal with it, and I know even less what would be suitable for everyone else. I'm mostly comparing to what I'm used to.

To clarify my position:

  • I have not converted my branch into a PR because it introduces a discrepancy between ./x and -- x completions. My personal opinion is (1) that the filtered out completion is better, and (2) that completion by splitted paths is more suitable for "diff". The latter could be a config param as you suggest.
  • After a double dash, I expect all git file completions to accept both ./x and x, but it seems hard to implement, even just for git diff. It may be out of the scope of this issue.
  • When a positional parameter starts like existing files, I'd expect git commands to complete with them, even if there is no "./" prefix or double dash. Eg. git diff src/ would complete.

@rsteube
Copy link
Member

rsteube commented Jan 5, 2024

Same, I'm limited to my own workflow so I don't encounter some edge cases.
Don't think I've grasped the whole picture yet, but regarding how to complete paths:

The last decision I had made here was that when the amount of values is

  • likely low (e.g. git add current changes) the full list should be used to access files quickly
  • likely high (e.g. git diff between distant refs) splitted paths should be used

Always using splitted paths for ... -- <TAB> might be worth giving a thought to.

@rsteube
Copy link
Member

rsteube commented Jan 6, 2024

@mytskine Updated positional completion fordiff and difftool.
I can see why I had not tackled that one yet, the variants are quite complex 😄 .
Give it a try and see if it works for you.

This was referenced Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants