On Tue, Oct 17, 2017 at 11:40:17AM +0900, Junio C Hamano wrote:
> Kevin Daudt <m...@ikke.info> writes:
> 
> >> +  setup_git_directory_gently(&nongit);
> >> +
> >> +  if (!nongit)
> >> +          malformed = (strbuf_check_branch_ref(&sb, arg) ||
> >> +                       !skip_prefix(sb.buf, "refs/heads/", &name));
> >> +  else
> >> +          malformed = check_branch_ref_format(arg);
> >> +
> >
> > Would it make sense to swap the logic and get rid of the double
> > negative (!nongit)?
> 
> I am trying to follow the pattern "handle the normal case that have
> been supported forever first, and then handle new exception next",
> so that it is easier to see that there is no behaviour change in the
> normal case, so I do not think it makes it easier to see to swap the
> if/else cases.

Ok, thanks for your reasoning, makes sense.
> >
> >> +  if (malformed)
> >>            die("'%s' is not a valid branch name", arg);
> >> -  printf("%s\n", sb.buf + 11);
> >> +  printf("%s\n", name);
> >> +  strbuf_release(&sb);
> >>    return 0;
> >>  }
> >>  

Reply via email to