Re: [PATCH 1/3] git-branch: add --track and --no-track options

Paolo Bonzini [off-list ref] writes:

quoted

Hmm. I think your use of 'mytopic' is very good for the purpose
of illustration. It makes clear which configuration takes name
from what. So I like your first paragraph. However, one reason
people like the separate remote layout is that it allows you to
name your own branch identically with that of the other side, so
in that sense, the description in your second paragraph matches
the real-life usage better. What I am getting at is that (1)
these two paragraphs are inconsistent, (2) there is a reason to
prefer the description in the first paragraph, and (3) there is
another reason to prefer the description in the second one.

Ok, I'll prefer using "mytopic" anywhere.

quoted

As Johannes already pointed this out, I think allowing this to
be controlled per remote is nice but overkill.  A single boolean
configuration, say "branch.autosetupmerge", would suffice.

In this case, patch 2/3 should also be withdrawn, right?

Do you mean you would agree that it is overkill?  Just in case;
you do not have to necessarily agree with me but convince me
your way, if you feel I am wrong.

If so, yeah, 2/3 needs a minor adjustment since the configuration
will not be on remote.* but one configuration variable.

Also I agree with many points Dscho made.  I understand you
agreed to avoid asprintf() from portability worries, which I
think is a sensible thing to do.

While I do not think we should avoid sscanf("%n"), I suspect
that the code in your patch is not helped by using it that much.

Do you mean you would agree that it is overkill?

It means I don't have much interest in trying to convince you your way.  I will try just once, and I have two arguments to make:

1) I added it in the first place because it made the design of patch 2/3 obvious.  If the shared configuration variable wins, I would withdraw patch 2/3 completely and just let the user use git-config (or "vi"...) to modify the default.

2) Also, I liked per-remote configuration because it gives you a quick view of which remotes you have just because you sometimes cherrypick from them, and which remotes you have because you are basing your work on them.

If you're convinced, I'll send the updated (final?) patch later today (which means you'll get it tomorrow morning in your timezone).  If you're not, I'm not sure I can update the patch today to use the shared configuration variable, but I'll get to that too.

Also I agree with many points Dscho made.  I understand you
agreed to avoid asprintf() from portability worries, which I
think is a sensible thing to do.

Sure, I was somehow convinced that git was already providing a portable version of it.

While I do not think we should avoid sscanf("%n"), I suspect
that the code in your patch is not helped by using it that much.

No, it's not.  But it's helped a lot by using sscanf itself, and "%n" is the only way I know to reliably test the return code of sscanf and, in the process, save one strchr and one strlen.

Paolo