On Fri, May 31, 2019 at 07:23:56PM +0200, Johannes Schindelin wrote:

Hi Denton,

On Fri, 31 May 2019, Denton Liu wrote:

quoted

On Fri, May 31, 2019 at 02:58:30PM +0200, Johannes Schindelin wrote:

quoted

On Thu, 30 May 2019, Denton Liu wrote:

[...]

quoted

quoted

quoted

I decided to go ahead and implement the includeIf onbranch semantics
for fun. For completeness, I'm sending it to the list but I'm not
really sure if this should get merged, since I don't really have a
use-case for this, especially if we go the branch-specific
format-patch config route.

Another thing to note is that this change doesn't completely cover
all the use-cases that the branch-specific format-patch does. In
particular, if I run

	$ git checkout foo
	$ git format-patch master..bar

with the `format.bar.*`, we'd get bar-specific configs, whereas with
`includeIf "onbranch:bar"`, we'd fail to include bar-specific configs
and, more dangerously, we'd be including foo's configs.

I actually think that this is fine. "on branch" means that you are on the
specified branch, not that you merely mention the branch name on the
command-line (in which case there would be the ambiguity "did the user
mean `master` or `bar`?").

The reason why I brought this up as a use case was because currently,
when format-patch generates a cover letter, with the above, it'll use
bar's branch description to populate it even if "foo" is checked out. As
a result, when implementing the branch-specific format-patch stuff, I
wanted to make this consistent so that we wouldn't end up in a situation
where the cover letter has the branch's description but is missing its
Cc's.

That strikes me as a different use case than `includeIf`. I could imagine
that you'd want a setting like `formatpatch.detecttargetbranch = auto` or
some such that would pick up the `format.bar*` settings if there was *one*
rev argument, and it was a commit range (or a tip commit), *and* it
obviously referred to a single target branch.

Correct. For context, upthread I initially implemented the
branch-specific format-patch configs but Ævar suggested that we
implement the onbranch config semantics instead.

So I was just addressing the fact that this patch can't supercede the
branch-specific format-patch stuff since they have different use cases
so the other patchset has to coexist with this one. I'm happy to see
that we're both in agreement about this.

It's just a scenario that is *very* specific to `git format-patch`.

For example, I would not, ever, want `git log ..next` to pick up a
config specific to `next` just because I mentioned a commit range with
`range` as the tip to start from.

Yeah, I'd never dream of implementing something that gross ;)

Ciao,
Dscho