All of lore.kernel.org
 help / color / mirror / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: Junio C Hamano <gitster@pobox.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 01:05:07 -0700	[thread overview]
Message-ID: <YFRbM1st0yINtScF@generichostname> (raw)
In-Reply-To: <xmqqsg4sryq9.fsf@gitster.g>

Hi Junio,

On Thu, Mar 18, 2021 at 01:30:38PM -0700, Junio C Hamano wrote:
> Denton Liu <liu.denton@gmail.com> writes:
> 
> > When completions are offered for `git stash branch<TAB>`, the user is
> > supposed to receive refs. This works in the case where the main git
> > command is called without arguments but if options are provided, such as
> > `git -C dir stash branch<TAB>`, then the `$cword -eq 3` provides
> > incorrect results.
> >
> > Count the words relative to the first instance of "stash" so that we
> > ignore arguments to the main git command.
> >
> > Unfortunately, this still does not work 100% correctly. For example, in
> > the case of something like `git -C stash stash branch<TAB>`, this will
> > incorrectly identify the first "stash" as the command. This seems to be
> > an edge-case that we can ignore, though, as other functions, such as
> > _git_worktree(), suffer from the same problem.
> 
> I am not familiar with how the completion support works, but doing
> this inside _git_stash() and still not being able to tell which
> "stash" on the command line is supposed to be the git subcommand
> smells quite fishy to me.  
> 
> How did the caller decide to invoke _git_stash helper function in
> the first place?
> 
> When it is given "git -C push --paginate stash branch<TAB>", it must
> have parsed the command line, past the options given to the "git"
> potty, to find "stash" on the command line that it is _git_stash and
> not _git_push that needs to be called, no?  If it were possible to
> propagate that information without losing it, then we do not have to
> recompute where the subcommand name is at all, do we?

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.

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.

Thanks,
Denton

  reply	other threads:[~2021-03-19  8:06 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 [this message]
2021-03-19 15:53         ` Junio C Hamano
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

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

  git send-email \
    --in-reply-to=YFRbM1st0yINtScF@generichostname \
    --to=liu.denton@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.