Re: [PATCH 1/7] builtin/verify-tag: stop using `the_repository`

[Patrick Steinhardt <ps@pks.im>]

On Mon, Feb 17, 2025 at 03:35:05PM +0530, Usman Akinyemi wrote:
> On Mon, Feb 17, 2025 at 12:25 PM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Sat, Feb 15, 2025 at 04:27:17AM +0530, Usman Akinyemi wrote:
> > > @@ -35,7 +34,8 @@ int cmd_verify_tag(int argc,
> > >               OPT_END()
> > >       };
> > >
> > > -     git_config(git_default_config, NULL);
> > > +     if (repo)
> > > +             repo_config(repo, git_default_config, NULL);
> > >
> >
> > I recently noticed that we have `usage_with_options_if_asked()`. Should
> > we use that function rather than making the call to `git_config()`
> > conditional? Otherwise it's not obvious why we have the conditional in
> > the first place.
> Hi Patrick,
> 
> I think the function is `show_usage_with_options_if_asked()`. The function
> is quite different from `git_config()` or the `repo_config()`.  The
> config function consults the configuration file for setting up config
> values and it uses the `repo` variable during this. While
> `show_usage_with_options_if_asked()` is used when the "-h" option is
> passed to the builtin functions to display the help string.
> 
> In a case when "-h" is passed to the builtin functions which use the
> RUN_SETUP macro, the `repo` config will be NULL.
> 
> There are some builtin commands functions that which has
> the`git_config()` function comes before
> `show_usage_with_options_if_asked()` or it's variant and some,
> `git_config()` comes after.
> 
> For those that have `git_config()` comes after
> `show_usage_with_options_if_asked()` , no need for the check, since
> the `show_usage_with_options_if_asked()`call will exit without
> reaching `git_config()`. For scenario where the `git_config()` comes
> earlier, we have to check the `repo` to see if it is NULL, if it is
> NULL, we are sure this happens when the "-h" is passed to the function
> and we do not need to setup and configuration since
> `show_usage_with_options_if_asked()` will exit.

Exactly, this is what my suggestion is. If we introduced new calls to
`show_usage_with_options_if_asked()` before `git_config()` we wouldn't
have to check for a `NULL` repository in the first place because we know
that we'd have already exited if there was a "-h" parameter.

Patrick