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

Complete bug IDs where appropriate #531

Merged
merged 5 commits into from
May 1, 2022

Conversation

krobelus
Copy link
Contributor

@krobelus krobelus commented Jan 24, 2021

This completes bug IDs in bash and fish. I haven't gotten completions to work in zsh.
In fish, the bug titles are shown as completion label, and can be searched for.
I don't know if/how other shells can show the completion label.
Maybe they don't use "\t" as separator but it seems to do no harm.

The interface does not yet seem ideal because sometimes subcommands and
bug IDs are both valid. Since there can be lots of bug IDs, they overshadow
the subcommands.
I wonder if instead of

git bug status open 0123acd

we should use

git bug status --open 0123acd

or if there's some other way to avoid ambiguity.

If that doesn't work out, fish has a -k/--keep-order flag, so subcommand
completions can be listed before bug IDs, but that's neither portable nor
a pretty solution.

Part of #493

@MichaelMure
Copy link
Collaborator

THIS IS AWESOME!

This completes bug IDs in bash and fish. I haven't gotten completions to work in zsh.

I think the ZSH completion code in cobra does support yet this new completion framework. I think it will work eventually, without any changes required on the git-bug side.

In fish, the bug titles are shown as completion label, and can be searched for.
I don't know if/how other shells can show the completion label.
Maybe they don't use "\t" as separator but it seems to do no harm.

I think the \t thing is to tell cobra about the label associated with the completion. If the shell doesn't support that, it will just get ignored.

The interface does not yet seem ideal because sometimes subcommands and
bug IDs are both valid. Since there can be lots of bug IDs, they overshadow
the subcommands.
I wonder if instead of

git bug status open 0123acd

we should use

git bug status --open 0123acd

or if there's some other way to avoid ambiguity.

Hmmm dunno. Even with changing status we'll have the problem with other commands that have subcommands. I think the docker completion has the same "issue" and it still works AFAIK. Now, if the completion order is not maintained between your code and the shell, maybe that's something worth reporting to https://github.com/spf13/cobra?

@krobelus
Copy link
Contributor Author

krobelus commented Jan 24, 2021

I'll push a minor fix - completions don't work yet for a command like git bug comment add -> fixed

@krobelus
Copy link
Contributor Author

krobelus commented Jan 24, 2021

Hmmm dunno. Even with changing status we'll have the problem with other commands that have subcommands. I think the docker completion has the same "issue" and it still works AFAIK.

Yeah, --open is probably no good, that was just a quick thought.
I can't think of a docker command that has the same issue. I think it's always be possible to know if the next argument is a container/image or something else.
Another way to avoid the ambiguity would be to split it into two subcommands, comment show and comment add, though that's ugly. Anyway, it's not a big deal, and I'm quite new to git-bug, so I'll have a better perspective as I use it more.

Now, if the completion order is not maintained between your code and the shell, maybe that's something worth reporting to https://github.com/spf13/cobra?

Ok, I'll report that. Looks like there are some other fish-specific issues over there that need addressing, I'll take care of those.
(Edit: not sorting doesn't seem to be easy to add with the way fish completions are implemented.. maybe I can think of a way)

@krobelus krobelus force-pushed the complete-bug-ids branch 2 times, most recently from 15cd8de to 8d23da6 Compare January 25, 2021 16:29
@krobelus
Copy link
Contributor Author

Ok, I've extended this to all subcommands and flags. This should cover all subcommands.
The label completion is especially clever, because it also takes into account labels which are already on the commandline.

In fish, git-bug ... completions work fine but git bug ... completions need a small wrapper, which will be merged soon (fish-shell/fish-shell#7652).. With that, fish completions can be enabled with
ln -s $PWD/misc/fish_completion/git-bug ~/.config/fish/completions/git-bug.fish

doc/gen_docs.go Outdated
@@ -21,22 +20,15 @@ func main() {
"Markdown": genMarkdown,
}

var wg sync.WaitGroup
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are you removing that? Fast build is nice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the commit message - without this commit I usually get a data race when running make.
I didn't really investigate what the real fix should be. Do you think we should use global variables for the Cobra commands?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's the opposite actually. Cobra use a global map here. This global map is shared even if multiple independent root command are created.

It looks like it could be easily solved in cobra though ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

well, not really easy but doable

@MichaelMure
Copy link
Collaborator

I was going to report the issue to cobra but I see you did it already ;)

I made it into its own issue though, with some extra possible solutions: spf13/cobra#1320

@MichaelMure
Copy link
Collaborator

Let's wait a bit to see if something is moving on the cobra side. We'll merge either way at the very last before the next version, don't worry.

@MichaelMure
Copy link
Collaborator

The cobra issue got resolved upstream, there is hope! spf13/cobra#1438

@6543
Copy link
Collaborator

6543 commented Aug 1, 2021

cobra bump is here ... need to be tested and then we can add it in ?!

@6543
Copy link
Collaborator

6543 commented Aug 1, 2021

ok tested and bump is in @krobelus can you rebase?

@MichaelMure
Copy link
Collaborator

hey hello,

I finally get more and more time to work on this project, sorry for the crazy long delay.

I've been trying out this PR to see if everything was fine. The build process got fixed indeed but ... the completion doesn't work for me. It might be because of the cobra upgrade as they made some related change, but they claim to have kept backward compatibility. I've looked around and everything seems fine.

Any idea? It's probably something silly.

@MichaelMure
Copy link
Collaborator

It's also possible that I'm just using it wrong.

@krobelus
Copy link
Contributor Author

krobelus commented Mar 2, 2022

fish and bash completions seem to work (although bash only works with git-bug, not git bug); added steps to the commit message. zsh didn't work on my brief attempt.

@MichaelMure
Copy link
Collaborator

@krobelus there is this little bash thing that should map git bug to git-bug for the completion, isn't that working? https://github.com/MichaelMure/git-bug/blob/master/commands/root.go#L54-L60

Maybe it's not being exported as part of the completion script anymore?

@MichaelMure
Copy link
Collaborator

I tried a few things, I can't get it to work with bash (I didn't try the others):

  • a simple source misc/bash_completion/git-bug bring the completion of the sub-command names, but not the options (--help and so on, which was working at some point), and git-bug select ^I gives filename completion.
  • your command bash --rcfile <(echo source misc/bash_completion/git-bug) brings a new shell but any attempt at completion yield bash: _get_comp_words_by_ref : command not found

Copy link
Collaborator

@MichaelMure MichaelMure left a comment

Choose a reason for hiding this comment

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

I'm trying to figure out what's happening. The following actually returns something that looks ok:

: git-bug __complete select 68
681b740	sqdsqdqsdqsdsq
e4c9831	sqdjlksqdsdiqolz
:0
Completion ended with directive: ShellCompDirectiveDefault

And yet, when I try git-bug select 68^I nothing happens.

completions[i] = bridge + "\t" + "Bridge"
}

return completions, cobra.ShellCompDirectiveDefault
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity, why not ShellCompDirectiveNoFileComp here?


func completeBug(env *Env) validArgsFunction {
return func(cmd *cobra.Command, args []string, toComplete string) (completions []string, directives cobra.ShellCompDirective) {
if len(args) > 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It'd be nice to complete even if len(args) == 0. That's one of the problem I had, I tried to complete git-bug select which didn't work as your code returns an empty array.
If you did that to avoid returning thousands of completion proposition, maybe we should have an arbitrary limit?

allIds := env.backend.AllBugsIds()
bugExcerpt := make([]*cache.BugExcerpt, len(allIds))
for i, id := range allIds {
var err error
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about filtering here to only output the ones with toCompleteas prefix?

@MichaelMure
Copy link
Collaborator

I found the root cause, GenBashCompletionV2 need to be called instead of the legacy one.

@krobelus
Copy link
Contributor Author

krobelus commented Mar 19, 2022 via email

Complete bug IDs, bridges, users, labels where appropriate.

This works in bash and fish. ZSH is not yet supported by Cobra.
In fish, descriptions (the part of a completion after the "\t") are shown
as completion label, and can be searched with Ctrl+S.

Reproduce with

	fish -C 'source misc/fish_completion/git-bug'
	git bug select ^I

(tested with fish version 3.3.1)

Also works with bash, but only for "git-bug" (with the dash)

	bash --rcfile <(echo source misc/bash_completion/git-bug)
	git-bug select ^I

Closes git-bug#493
@MichaelMure
Copy link
Collaborator

With @tst2005 fix for bash and a bunch of small adjustments I think we have something really cool!

Thanks @krobelus, this is awesome.

@MichaelMure MichaelMure merged commit 547d626 into git-bug:master May 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants