Re: [PATCH 1/9] init: allow overriding the default branch name for new repositories

> On Wed, Jun 10, 2020 at 5:19 PM Don Goodman-Wilson via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > [...]
> > To make this process much less cumbersome, let's introduce support for
> > `core.defaultBranchName`. That way, users won't need to keep their
> > copied template files up to date, and won't interfere with default hooks
> > installed by their administrators.
> > [...]
> > Signed-off-by: Don Goodman-Wilson <don@goodman-wilson.com>
> > ---
> > diff --git a/refs.c b/refs.c
> > @@ -560,6 +560,40 @@ void expand_ref_prefix(struct argv_array *prefixes, const char *prefix)
> > +                       die(_("Could not retrieve `core.defaultBranchName`"));
>
> Nit: here the error message is capitalized...

> > +               if (from_config)
> > +                       branch_name = from_config;
> > +               else
> > +                       branch_name = "master";
>
> Non-actionable nit: could be written:
>
>     branch_name = from_config ? from_config : "master";

> > +       }
> > +
> > +       if (short_name)
> > +               return from_config ? from_config : xstrdup(branch_name);
>
> The logic overall is a bit difficult to follow when trying to
> understand when and when not to duplicate the string and when and when
> not to free(), but seems to be correct.

> > +       /* prepend "refs/heads/" to the branch name */
> > +       prefixed = xstrfmt("refs/heads/%s", branch_name);
> > +       if (check_refname_format(prefixed, 0))
> > +               die(_("invalid default branch name: '%s'"), branch_name);
>
> Here, the error message is not capitalized. It would be nice for both
> messages to share a common capitalization scheme. These days, we tend
> to favor _not_ capitalizing error messages, so perhaps remove
> capitalization from the earlier one.
>
> > +/*
> > + * Retrieves the name of the default branch. If `short_name` is non-zero, the
> > + * branch name will be prefixed with "refs/heads/".
> > + */
> > +char *git_default_branch_name(int short_name);
>
> Overall, the internal logic regarding duplicating/freeing strings
> would probably be easier to grok if there were two separate functions:
>
>     char *git_default_branch_name(void);
>     char *git_default_ref_name(void);
>
> but that's subjective.