git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Denton Liu <liu.denton@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [RESEND PATCH 2/3] git-completion.bash: fix `git <args>... stash branch` bug
Date: Fri, 19 Mar 2021 08:53:50 -0700	[thread overview]
Message-ID: <xmqqy2ejnnqp.fsf@gitster.g> (raw)
In-Reply-To: <YFRbM1st0yINtScF@generichostname> (Denton Liu's message of "Fri, 19 Mar 2021 01:05:07 -0700")

Denton Liu <liu.denton@gmail.com> writes:

> Good observation. _git_stash() is called in the body of
> __git_complete_command() which is called by __git_main(). There is
> currently no mechanism by which to pass the index of the command over to
> _git_*() completion functions.

I think, given that "set | grep _git" tells us we use many globals
already, it would be OK to introduce another variable, call it
$__git_subcmd_pos, and assign to it when the command dispatcher
discovers which token on the command line is the subcommand name and
decides to call the subcommand specific completion helper function.

Or does the command dispatcher not exactly know the position
(e.g. iterates with "for w" and knows $w==stash in the current
iteration but it is not counting the position in the array)?  If so,
then we'd need a surgery larger than that.

But if we only need to set a variable, we won't have to change the
calling convention of these helpers (well, we shouldn't be changing
the arguments to completion functions lightly anyway---third-party
completion functions can be called from __git_complete_command, if I
am reading the code correctly, and we cannot update them all even if
we wanted to).

And most subcommands that do not care where on the command line the
subcommand name is won't have to change anything.

> That being said, passing in the index to all functions would definitely
> be doable. I can work on a series in the future that passes in the index
> of the command so that working with $cword is more robust but I'd prefer
> if that were handled outside this series to keep it focused.

If the breakage of "stash branch" were a serious show-stopper bug
that needs to be fixed right away, I would agree that a band-aid
solution that would work most of the time would be fine, but I
didn't get an impression that it is so urgent and we can afford to
fix it right this time, together with the other completion that
share the same problem (you mentioned _git_worktree IIRC).

Thanks.

  reply	other threads:[~2021-03-19 15:54 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-16  0:54 [PATCH 0/3] git-completion.bash: improvements to _git_stash() Denton Liu
2021-03-16  0:54 ` [PATCH 1/3] git-completion.bash: extract from else in _git_stash() Denton Liu
2021-03-16  0:54 ` [PATCH 2/3] git-completion.bash: fix `git <args>... stash branch` bug Denton Liu
2021-03-16  0:54 ` [PATCH 3/3] git-completion.bash: use __gitcomp_builtin() in _git_stash() Denton Liu
2021-03-18  9:46 ` [RESEND PATCH 0/3] git-completion.bash: improvements to _git_stash() Denton Liu
2021-03-18  9:46   ` [RESEND PATCH 1/3] git-completion.bash: extract from else in _git_stash() Denton Liu
2021-03-18  9:46   ` [RESEND PATCH 2/3] git-completion.bash: fix `git <args>... stash branch` bug Denton Liu
2021-03-18 20:30     ` Junio C Hamano
2021-03-19  8:05       ` Denton Liu
2021-03-19 15:53         ` Junio C Hamano [this message]
2021-03-18  9:46   ` [RESEND PATCH 3/3] git-completion.bash: use __gitcomp_builtin() in _git_stash() Denton Liu
2021-03-18 21:58   ` [RESEND PATCH 0/3] git-completion.bash: improvements to _git_stash() Junio C Hamano
2021-03-19  8:09     ` Denton Liu
2021-03-19 15:57       ` Junio C Hamano
2021-03-24  8:36 ` [PATCH v2 " Denton Liu
2021-03-24  8:36   ` [PATCH v2 1/3] git-completion.bash: pass $__git_subcommand_idx from __git_main() Denton Liu
2021-03-27 18:35     ` SZEDER Gábor
2021-03-28 10:31     ` SZEDER Gábor
2021-03-24  8:36   ` [PATCH v2 2/3] git-completion.bash: extract from else in _git_stash() Denton Liu
2021-03-28 10:30     ` SZEDER Gábor
2021-03-24  8:36   ` [PATCH v2 3/3] git-completion.bash: use __gitcomp_builtin() " Denton Liu
2021-03-28 11:04     ` SZEDER Gábor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy2ejnnqp.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=liu.denton@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).