Kaartic Sivaraam <kaartic.sivar...@gmail.com> writes:

> diff --git a/builtin/branch.c b/builtin/branch.c
> index 7018e5d75..c2bbf8c3d 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -458,11 +458,42 @@ static void reject_rebase_or_bisect_branch(const char 
> *target)
>       free_worktrees(worktrees);
>  }
>  
> +static void get_error_msg(struct strbuf* error_msg, const char* oldname, 
> unsigned old_branch_exists,
> +                       const char* newname, enum branch_validation_result 
> res)
> +{
> +     const char* connector_string = _(", and ");
> +
> +     if (!old_branch_exists) {
> +             strbuf_addf(error_msg, _("branch '%s' doesn't exist"), oldname);
> +     }

No {} around a single statement block of "if", especially when there
is no "else" that has multi-statement block that needs {}.

> +     switch (res) {
> +             case BRANCH_EXISTS_NO_FORCE:
> +                     strbuf_addf(error_msg, "%s", (!old_branch_exists) ? 
> connector_string : "");
> +                     strbuf_addf(error_msg,_("branch '%s' already exists"), 
> newname);
> +                     break;

The case arms and their statements are indented by one level too much.
The lines are getting overlong.  Find a good place to split, e.g.

                strbuf_addf(error_msg, "%s",
                        !old_branch_exists ? connector_string : "");

Leave a single SP after each "," in an arguments list.

As Eric pointed out, this certainly smells like a sentence lego that
we would be better off without.

>  static void copy_or_rename_branch(const char *oldname, const char *newname, 
> int copy, int force)
>  {
>       struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = 
> STRBUF_INIT;
>       struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
>       int recovery = 0;
> +     struct strbuf error_msg = STRBUF_INIT, empty = STRBUF_INIT;
> +     enum branch_validation_result res;
>  
>       if (!oldname) {
>               if (copy)
> @@ -471,15 +502,13 @@ static void copy_or_rename_branch(const char *oldname, 
> const char *newname, int
>                       die(_("cannot rename the current branch while not on 
> any."));
>       }
>  
> -     if (strbuf_check_branch_ref(&oldref, oldname)) {
> +     if (strbuf_check_branch_ref(&oldref, oldname) && ref_exists(oldref.buf))
> +     {

Opening brace { that begins a block comes at the end of the line
that closes the condition of "if"; if you found that your line is
overlong, perhaps do it like so instead:

        if (strbuf_check_branch_ref(&oldref, oldname) &&
            ref_exists(oldref.buf)) {

Reply via email to