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

Add completions to some git stash sub-commands #1216

Closed
wants to merge 7 commits into from

Conversation

mandeepsandhu
Copy link
Contributor

Issue #1102: For some git stash commands, we need to dynamically
provide the stash IDs of the saved stashes as completions.

This commit provides support for the apply, branch, drop, pop and
show stash sub-commands.

@@ -51,7 +51,7 @@ end

function __fish_git_using_command
set cmd (commandline -opc)
if [ (count $cmd) -gt 1 ]
if [ (count $cmd) -eq 2 ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change breaks completion for (e.g.) git rebase with more than one branch as an argument.

I think you'd be better off leaving this alone, and instead using __fish_git_using_command stash; and __fish_git_stash_using_command apply.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! Reverted this change.

Issue fish-shell#1102: For some git stash commands, we need to dynamically
provide the stash IDs of the saved stashes as completions.

This commit provides support for the apply, branch, drop, pop and
show stash sub-commands.
@zanchey
Copy link
Member

zanchey commented Jan 9, 2014

This is almost there, but now I still get completions e.g for 'list' when I have already typed git stash apply.

It would also be useful to make the stash description the name of the stash.

@mandeepsandhu
Copy link
Contributor Author

I was away on vacation so could not test this.

Yes, I too see this behavior when testing with the latest commits from 3 weeks back. However, today after updating with the latest master, the completion has stopped working altogether. It only works if at least 1 character is typed and then too as you noticed, it's showing more options than expected. I'm looking into it right now.

I'll look into providing the stash desc as it's name. I'm guessing you want the description to be shown where currently we just write 'Stash', correct? (the name still remains stash@{0} etc)

@mandeepsandhu
Copy link
Contributor Author

I tried a build of a clean upstream master (w/o any of my changes) and there too I see the problem where git stash completion stops if no input is given. Eg the following does not show any options, where previously it used to show the stash sub-commands:

$ git stash [press tab]

It looks like a breakage as previously this functionality was working.

I tried using git bisect to narrow it down to the problematic commit but soon ran into compilation errors with some of the commits (BTW, isn't master branch supposed to contain compilable/working code??).

I have a feeling its related to the _parse tree_ rework that ridiculous fish is doing (but I could be completely wrong here), as that's where bisect roughly pointed me to.

@KamilaBorowska
Copy link
Contributor

(BTW, isn't master branch supposed to contain compilable/working code??)

Yes, but what you most likely see is the code that wasn't in master to begin with, but appeared after merge.

@zanchey
Copy link
Member

zanchey commented Jan 24, 2014

It should be fixed with #1261.

@mandeepsandhu
Copy link
Contributor Author

Ok, I'll wait for #1261 to get merged. In the meantime I'll get the listing of stash desc alongwith the ID/name working.

One question here. Should we elide the desc text if it's too long (append "...") ?

@mandeepsandhu
Copy link
Contributor Author

I just checked and it seems that we cannot give a separate description string for each suggestion (args to complete 's -a option). Whatever is given to -d opt, is taken as-is and shown along with each suggestion. So with this limitation, I don't think we can provide a per-stash description string. Or am I missing something?

@zanchey
Copy link
Member

zanchey commented Jan 25, 2014

Dynamically-generated completions can set the description for a completion by outputting option<TAB>description. This is not documented, which is a problem, but is used e.g. by __fish_complete_users.

@zanchey
Copy link
Member

zanchey commented Jan 25, 2014

Also, fish automatically truncates long descriptions; there's no need to do it manually.

Issue fish-shell#1102: Addressed review comments.

Now we are also listing the per-stash description alongwith with
stash ref log IDs.
@mandeepsandhu
Copy link
Contributor Author

Thanks for pointing that out David. I've changed the suggestion function in git.fish. Now it's showing the per-stash description along with the stash IDs.

As to truncating of the desc, I had a decently long text in one of the stash descriptions which was not elided (exact desc: WIP on rebase-test: 2dca46c Added D to junk.txt in rebase branch). Any idea after how many characters we start truncating?

I'll wait for #1261 to get merged and test out my changes again with it.

@zanchey
Copy link
Member

zanchey commented Jan 29, 2014

That's been merged, so give it another go. The truncation happens dynamically based on window size and number of columns in the completions.

@mandeepsandhu
Copy link
Contributor Author

I updated my branch to the latest from master and got the changes for #1261.

There are 2 issues:

  1. On pressing <tab> after typing git stash apply, extra suggestions are being given by __fish_git_using_command stash which succeeds the test for this command string. However, we don't want to consider this completion suggestion as we are now, only interested in the sub-command completions. For this to work we shouldn't be returning 0 if a sub-command string is provided. We can solve this by introducing a new function, say __fish_git_not_using_subcommand which returns 1 if arg count is > 2 and combine it with __fish_git_using_command stash. (eg __fish_git_using_command stash; and __fish_git_stash_not_using_subcommand. What do you think?
  2. If I type git stash apply st [tab], then initially I get suggestions for the various stash' (as stash@\{0\} etc), but on pressing <tab> subsequently, it no longer cycles through the options as it used to before. Could this be related to Regression: git TAB completions no longer work reliably #1261 as well?

If you have time, can you try it on your setup too? Thanks.

@mandeepsandhu
Copy link
Contributor Author

David, did you get a chance to try out the tab completion problem I mentioned in (2) above?

Issue fish-shell#1102: Addressed review comments.

Fixed issue where other subcommands were being suggested alongwith
the stash IDs.
@zanchey
Copy link
Member

zanchey commented Feb 4, 2014

Yes, this looks like a regression; see #1282.

@mandeepsandhu
Copy link
Contributor Author

David, I've tested my changes with new pager (by setting set fish_new_pager 1). It works fine. Can you review the changes?

return 0
end

function __fish_git_stashs_and_desc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style point: this function would be better off as __fish_git_complete_stashes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@zanchey
Copy link
Member

zanchey commented Feb 16, 2014

This still doesn't work for me, but I'm not sure why. When I type git stash apply and push tab, I'm still offered the general stash subcommands.

@zanchey
Copy link
Member

zanchey commented Feb 16, 2014

Whoops, my standard completions were still getting loaded.

Works beautifully. Thanks for your patience and your contribution!

Merged with squash and rebase here:
To git@github.com:fish-shell/fish-shell.git
64ca6c0..ef9f2ab master -> master

@zanchey zanchey closed this Feb 16, 2014
@mandeepsandhu
Copy link
Contributor Author

Thanks David. I'm just glad I could help. I love fish! :)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants