Re: [Outreachy PATCH v3 1/2] parse-options: answer --git-completion-helper before command runs

From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Oct 30, 2026, 09:44 UTC
Message-ID: <123de4e4-3387-4eef-8a05-9ed1ad1fce30@gmail.com>
In-Reply-To: <20261029231621.48958-1-tangajoris@gmail.com>

Hi Tanga Joris,

Just a gentle reminder about this portion of the MyFirstContribution[1]:

  It’s good manners to reply to each comment in the mailing list
  discussion instead of letting the next version of your patch be your
  only response. Tell the reviewer whether you plan to make the
  suggested change, keep the original, or pursue a different approach.
  This way reviewers can respond to your reasoning before you spend time
  preparing a version they may not agree with, and later do not need to
  inspect your v2 to figure out whether you implemented their comment or
  not.

On 10/30/26 04:46, Tanga Joris wrote:

Show 8 quoted lines

> fast-import parses its options only after reading the "feature"
> lines from its input, and it creates a temporary pack file before
> that. So "git fast-import --git-completion-helper" waits for input,
> and each run leaves a tmp_pack_* file behind.
> > bash completion runs the helper with the terminal as input, so
> "git fast-import --<TAB>" would freeze once fast-import is no
> longer marked with the NO_PARSEOPT flag.
> May be give a hint that you plan to do this in the following commit? That might give someone without context (for example someone reading the commit after a while) a hint about why you are worrying about marking fast-import with the NO_PARSEOPT flag.

Show 33 quoted lines

> Since show_usage_with_options_if_asked() already answers -h and
> --help-all before a command does any work, it now also answers
> the completion helper, so fast-import and other commands that
> call it reply without reading input.
> > Reported-by: René Scharfe <l.s.r@web.de>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Tanga Joris <tangajoris@gmail.com>
> ---
> Changes since v2:
>   - Moved the check from fast-import into
>     show_usage_with_options_if_asked() in parse-options,
>     as Junio suggested.
>   - 2/2 is unchanged
> >   parse-options.c        | 6 ++++++
>   t/t9300-fast-import.sh | 8 ++++++++
>   2 files changed, 14 insertions(+)
> > diff --git a/parse-options.c b/parse-options.c
> index 283a0f1dc2..f2a23b79a4 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -1536,6 +1536,12 @@ void show_usage_with_options_if_asked(int ac, const char **av,
>   			usage_with_options_internal(NULL, usagestr, opts,
>   						    USAGE_FULL, USAGE_TO_STDOUT);
>   			exit(0);
> +		} else if (!strcmp(av[1], "--git-completion-helper")) {
> +			show_gitcomp(opts, 0);
> +			exit(0);
> +		} else if (!strcmp(av[1], "--git-completion-helper-all")) {
> +			show_gitcomp(opts, 1);
> +			exit(0);
>   		}
>   	}
>   }

The change seems to be fine but I wonder if it is worth a note in parse-options.h about why show_usage_with_options_if_asked() is also handling the '--git-completion-helper' and '--git-completion-helper-all' arguments?

[ References ]

[1]: https://git-scm.com/docs/MyFirstContribution#reviewing

-- 
Sivaraam