Re: [PATCH v2 1/2] branch: suggest <remote>/<branch> on upstream slip

[Junio C Hamano <gitster@pobox.com>]

Harald Nordgren <haraldnordgren@gmail.com> writes:

>> Do we still need the _if_enabled() thing here?  Isn't the caller
>> gated with the same condition in this version?
>>
>> > +     strbuf_release(&remote_ref);
>> > +     exit(code);
>> > +}
>> > +
>> >  int cmd_branch(int argc,
>> >              const char **argv,
>> >              const char *prefix,
>> > @@ -957,6 +980,9 @@ int cmd_branch(int argc,
>> >               if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
>> >                       if (!argc || branch_checked_out(branch->refname))
>> >                               die(_("no commit on branch '%s' yet"), branch->name);
>> > +                     if (argc == 1 &&
>> > +                         advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
>> > +                             die_if_upstream_looks_like_remote(new_upstream, argv[0]);
>> >                       die(_("branch '%s' does not exist"), branch->name);
>> >               }
>
> I think we do, so it will give the advice and tell the user that it
> can be disabled in the standard format.

I was hoping that unconditional advise() should be sufficient, but
the caller there needs to say if_enabled, even though it _knows_
that it is enabled, only to give the turn-off instructions.

I wonder if future readers would be confused just like I was,
without a comment on the callsite of _if_enabled() added by this
patch?

Thanks.