Re: [PATCH 3/3] rev-list --min-parents,--max-parents: doc and test and completion

Michael J Gruber [off-list ref] writes:

Jeff King venit, vidit, dixit 21.03.2011 11:54:

quoted

On Mon, Mar 21, 2011 at 10:01:53AM +0100, Michael J Gruber wrote:

In my community it is very common, which may partly be due to the fact
that there is a strong proportion of non-native speakers. It took it for
granted that it's a standard expression.

Perhaps in math circles or something?

quoted

I really think "Show only commits which have at least (or at most,
respectively) that many commits" says the same thing, but is way more
accessible.

Sounds good, I'm happy with that. Resend or squash on apply?

The "... A resp. B" composition did made me go "huh?", even though I
managed to guess what you meant; I agree Jeff's rewrite is much more
approacheable.

The code part of the patch needs a bit of touch-up anyway, so let's do a
v2.

It is not like you will have a million copies of "struct rev_info", while
you will be reading from and comparing with the field for each commit
during the traversal, so I'd rather see these max/min stored in the usual
"int", not squashed into bitfields.  Also initialize max to the magic
token that means "unlimited" in init_revisions() without swapping the
meaning of comparison.  One initialization assignment you can omit there
is not worth the resulting confusion.

quoted

quoted

quoted

That way it is obvious that "--merges" cancels a previous --min-parents
on the command line (maybe the text should be "this is an alias for..."
to make it clear that doing it is exactly the same).

Yes, that is helpful. I have doubts about "alias" for. Without wanting
to sound elitist or something, I have the impression that we start
catering for users who understand "equivalent" more reliably than "alias".

I just wanted to make sure people didn't think "equivalent" meant "has a
similar effect to" as opposed to "is exactly as if you did". But reading
it again, I think "equivalent" is fine, and I see you picked it up in
the latest series.

I may be wrong about what is common in this case, too. For me, "alias"
is foremost a technical term, and I would guess that many non-native
speaker know "alias" either in the technical sense or not at all, but
not so much in the common English sense. But either way is fine.

We would want to make sure the reader understands that saying --no-merges
is _exactly the same_ (your words above) as saying --max-parents=1, so why
not spell that out, i.e. "This is exactly the same as `--max-parents=1`"?

Thanks.

Junio C Hamano venit, vidit, dixit 21.03.2011 15:54:

Michael J Gruber [off-list ref] writes:

quoted

Jeff King venit, vidit, dixit 21.03.2011 11:54:

quoted

On Mon, Mar 21, 2011 at 10:01:53AM +0100, Michael J Gruber wrote:

In my community it is very common, which may partly be due to the fact
that there is a strong proportion of non-native speakers. It took it for
granted that it's a standard expression.

Perhaps in math circles or something?

Yeah, those (us) weirdos ;)

quoted

quoted

I really think "Show only commits which have at least (or at most,
respectively) that many commits" says the same thing, but is way more
accessible.

Sounds good, I'm happy with that. Resend or squash on apply?

The "... A resp. B" composition did made me go "huh?", even though I
managed to guess what you meant; I agree Jeff's rewrite is much more
approacheable.

The code part of the patch needs a bit of touch-up anyway, so let's do a
v2.

It is not like you will have a million copies of "struct rev_info", while
you will be reading from and comparing with the field for each commit
during the traversal, so I'd rather see these max/min stored in the usual
"int", not squashed into bitfields.  Also initialize max to the magic
token that means "unlimited" in init_revisions() without swapping the
meaning of comparison.  One initialization assignment you can omit there
is not worth the resulting confusion.

quoted

quoted

quoted

quoted

That way it is obvious that "--merges" cancels a previous --min-parents
on the command line (maybe the text should be "this is an alias for..."
to make it clear that doing it is exactly the same).

Yes, that is helpful. I have doubts about "alias" for. Without wanting
to sound elitist or something, I have the impression that we start
catering for users who understand "equivalent" more reliably than "alias".

I just wanted to make sure people didn't think "equivalent" meant "has a
similar effect to" as opposed to "is exactly as if you did". But reading
it again, I think "equivalent" is fine, and I see you picked it up in
the latest series.

I may be wrong about what is common in this case, too. For me, "alias"
is foremost a technical term, and I would guess that many non-native
speaker know "alias" either in the technical sense or not at all, but
not so much in the common English sense. But either way is fine.

We would want to make sure the reader understands that saying --no-merges
is _exactly the same_ (your words above) as saying --max-parents=1, so why
not spell that out, i.e. "This is exactly the same as `--max-parents=1`"?

Thanks.

I guess you haven't seen v2 yet, where most of these changes are
incorporated.

For the doc, "exactly the same" is clean and simple and understandable
for both Jeff and myself, I guess ;)

Michael