git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Denton Liu <liu.denton@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 2/5] git-completion.bash: rename to $__git_cmd_idx
Date: Tue, 20 Apr 2021 23:14:32 +0200	[thread overview]
Message-ID: <20210420211432.GA3842406@szeder.dev> (raw)
In-Reply-To: <xmqqr1j44r5g.fsf@gitster.g>

On Tue, Apr 20, 2021 at 01:50:19PM -0700, Junio C Hamano wrote:
> Denton Liu <liu.denton@gmail.com> writes:
> 
> > In e94fb44042 (git-completion.bash: pass $__git_subcommand_idx from
> > __git_main(), 2021-03-24), the $__git_subcommand_idx variable was
> > introduced. Naming it after the index of the subcommand is flat-out
> > wrong as this variable really holds the index of the git comand (e.g.
> 
> comand -> command
> 
> > "stash").
> >
> > Rename this variable so that it's obvious it's about git commands. While
> > we're at it, shorten up its name so that it's still readable without
> > being a handful to type.
> 
> As the patch has already written, I won't complain too much, but to
> many people "git" is a command and "add", "commit" etc. are
> subcommands of "git", so I do not see git_subcommand_idx so wrong
> that it needs to be renamed.  I do understand that it is a bit too
> long and it may be easier to type if renamed to say git_cmd_idx,
> though ;-)

The completion functions for git commands having subcommands usually
start like this:

    _git_remote ()
    {
        local subcommands="
                add rename remove set-head set-branches
                get-url set-url show prune update
                "
        local subcommand="$(__git_find_on_cmdline "$subcommands")"
        if [ -z "$subcommand" ]; then

__git_subcommand_idx holds the index of the word "remote", not the
index of "add/rename/etc.", so in the context of the completion script
that name is misleading.


  reply	other threads:[~2021-04-20 21:14 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-20  9:19 [PATCH 0/5] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
2021-04-20  9:19 ` [PATCH 1/5] git-completion.bash: separate some commands onto their own line Denton Liu
2021-04-20  9:19 ` [PATCH 2/5] git-completion.bash: rename to $__git_cmd_idx Denton Liu
2021-04-20 20:50   ` Junio C Hamano
2021-04-20 21:14     ` SZEDER Gábor [this message]
2021-04-20 22:31       ` Junio C Hamano
2021-04-20  9:19 ` [PATCH 3/5] git-completion.bash: use $__git_cmd_idx in more places Denton Liu
2021-04-20  9:19 ` [PATCH 4/5] git-completion.bash: consolidate cases in _git_stash() Denton Liu
2021-04-20 10:44   ` Ævar Arnfjörð Bjarmason
2021-04-21  4:03     ` Denton Liu
2021-04-20  9:19 ` [PATCH 5/5] git-completion.bash: consolidate no-subcommand case for _git_stash() Denton Liu
2021-04-20 21:10   ` Junio C Hamano
2021-04-21  4:08     ` Denton Liu
2021-04-22 10:00 ` [PATCH v2 0/4] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
2021-04-22 10:00   ` [PATCH v2 1/4] git-completion.bash: separate some commands onto their own line Denton Liu
2021-04-22 10:00   ` [PATCH v2 2/4] git-completion.bash: rename to $__git_cmd_idx Denton Liu
2021-04-22 10:00   ` [PATCH v2 3/4] git-completion.bash: use $__git_cmd_idx in more places Denton Liu
2021-04-22 10:00   ` [PATCH v2 4/4] git-completion.bash: consolidate cases in _git_stash() Denton Liu

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=20210420211432.GA3842406@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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.
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 NNTP newsgroup(s).