On 8/10/22 6:17, Junio C Hamano wrote:
> Rubén Justo <rjusto@xxxxxxxxx> writes:
>
>> } else if (new_upstream) {
>> - struct branch *branch = branch_get(argv[0]);
>> + struct branch *branch;
>> + struct strbuf buf = STRBUF_INIT;
>>
>> - if (argc > 1)
>> + if (!argc)
>> + branch = branch_get(NULL);
>> + else if (argc == 1) {
>> + strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
>> + branch = branch_get(buf.buf);
>> + } else
>> die(_("too many arguments to set new upstream"));
>>
>> if (!branch) {
>> @@ -854,11 +867,17 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>> dwim_and_setup_tracking(the_repository, branch->name,
>> new_upstream, BRANCH_TRACK_OVERRIDE,
>> quiet);
>> + strbuf_release(&buf);
>> } else if (unset_upstream) {
>> - struct branch *branch = branch_get(argv[0]);
>> + struct branch *branch;
>> struct strbuf buf = STRBUF_INIT;
>>
>> - if (argc > 1)
>> + if (!argc)
>> + branch = branch_get(NULL);
>> + else if (argc == 1) {
>> + strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
>> + branch = branch_get(buf.buf);
>> + } else
>> die(_("too many arguments to unset upstream"));
>
> The above two are a bit repetitious. A helper like
>
> static struct branch *interpret_local(int ac, const char **av)
> {
> struct branch *branch;
> if (!ac) {
> branch = branch_get(NULL);
> } else if (ac == 1) {
> struct strbuf buf = STRBUF_INIT;
> strbuf_branchname(&buf, av[0], INTERPRET_BRANCH_LOCAL);
> branch = branch_get(buf.buf);
> strbuf_release(&buf);
> } else {
> die(_("too many arguments"));
> }
> return branch;
> }
>
> might be useful, and it frees the callers from worrying about
> temporary allocations.
Yes, it leaves a repetitive taste. I thought about joining the two cases, but
the taste with multiple if/else was worse. Following what you suggest with the
"too many arguments" error, I'll try to reduce that repetitive taste.
Thanks
>
> But the code written is OK as-is.
>
