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 reset` completions issues #4329

Closed
mqudsi opened this Issue Aug 14, 2017 · 7 comments

Comments

Projects
None yet
2 participants
@mqudsi
Contributor

mqudsi commented Aug 14, 2017

I'm pretty sure the git completion is one of the more complicated ones, so please let me know if more information is needed.

Before anything else, here's my output of git status:

On branch master
Your branch is behind 'origin/master' by 1 commit, and can be fast-forwarded.
  (use "git pull" to update your local branch)
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   Makefile

If I type in

git reset M<tab>

the M gets expanded to/replaced with master (no other option is available).

If I type in

git reset Make<tab>

the Make gets expanded to/replaced with HEAD@\{0\} (literally, with the escapes)

several issues:

  • file completions should be supported for git reset
  • shouldn't adding letters to the argument being completed narrow down the list of completion options, not change them entirely?

@mqudsi mqudsi changed the title from git reset completions override correct options to git reset completions issues Aug 14, 2017

@mqudsi mqudsi added the completions label Aug 14, 2017

@mqudsi mqudsi changed the title from git reset completions issues to `git reset` completions issues Aug 14, 2017

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Aug 14, 2017

Member

file completions should be supported for git reset

Yes, though I'd argue it should be modified files. Ideally files modified since the "tree-ish" argument to reset, but that's complicated. It's almost trivial to add files modified since the last commit, since we already have that function (for git checkout).

the Make gets expanded to/replaced with HEAD@{0} (literally, with the escapes)

That's a reflog entry.

The escapes are to be expected - without them, HEAD@{0} expands the braces and ends up as HEAD@0, which is invalid.

The issue here is that it's matching the description as well (see git reflog. I'm betting that includes the word "Make"). That is behaving as expected - it'll only ever do that when there's no other match. We just need to add those matches.

Member

faho commented Aug 14, 2017

file completions should be supported for git reset

Yes, though I'd argue it should be modified files. Ideally files modified since the "tree-ish" argument to reset, but that's complicated. It's almost trivial to add files modified since the last commit, since we already have that function (for git checkout).

the Make gets expanded to/replaced with HEAD@{0} (literally, with the escapes)

That's a reflog entry.

The escapes are to be expected - without them, HEAD@{0} expands the braces and ends up as HEAD@0, which is invalid.

The issue here is that it's matching the description as well (see git reflog. I'm betting that includes the word "Make"). That is behaving as expected - it'll only ever do that when there's no other match. We just need to add those matches.

@faho faho added the bug label Aug 14, 2017

@faho faho added this to the fish 2.7.0 milestone Aug 14, 2017

@mqudsi

This comment has been minimized.

Show comment
Hide comment
@mqudsi

mqudsi Aug 14, 2017

Contributor

OK, that explains why m didn't match HEAD and Make did. The first had a match (master) so the commit text wasn't being searched.

Contributor

mqudsi commented Aug 14, 2017

OK, that explains why m didn't match HEAD and Make did. The first had a match (master) so the commit text wasn't being searched.

faho added a commit to faho/fish-shell that referenced this issue Aug 14, 2017

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Aug 14, 2017

Member

@mqudsi: See my git-reset branch. I'm holding off messing with master for a while.

Member

faho commented Aug 14, 2017

@mqudsi: See my git-reset branch. I'm holding off messing with master for a while.

@mqudsi

This comment has been minimized.

Show comment
Hide comment
@mqudsi

mqudsi Aug 14, 2017

Contributor

Does __fish_git_modified_files include deleted?

Contributor

mqudsi commented Aug 14, 2017

Does __fish_git_modified_files include deleted?

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Aug 14, 2017

Member

Yes. It uses git diff --name-only under the covers.

Member

faho commented Aug 14, 2017

Yes. It uses git diff --name-only under the covers.

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Aug 14, 2017

Member

Though I think we might need another argument here. git reset changes the index ("staging area"). git checkout changes the working tree (the actual directory).

Member

faho commented Aug 14, 2017

Though I think we might need another argument here. git reset changes the index ("staging area"). git checkout changes the working tree (the actual directory).

faho added a commit to faho/fish-shell that referenced this issue Aug 14, 2017

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Aug 14, 2017

Member

Okay, git diff --cached should do the trick.

Member

faho commented Aug 14, 2017

Okay, git diff --cached should do the trick.

@faho faho self-assigned this Aug 14, 2017

faho added a commit to faho/fish-shell that referenced this issue Sep 6, 2017

git completions: Use modified files in the index for `reset`
Fixes fish-shell#4329.

(cherry picked from commit 9765e861306fd2851f530a27e8eb778c1682a7c8)

@faho faho closed this in c2f0a45 Sep 6, 2017

zanchey added a commit that referenced this issue Sep 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment