Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly

[Ramkumar Ramachandra <artagnon@gmail.com>]

Phil Hord wrote:
> References to git-log seem out of place to me here in git-diffcore.  I
> know it's only an example, but it seems that Git normally describes
> these 'reference selectors' more generically.  The generic description
> may be more confusing to new users, but this patch is not the place to
> consider whether it should change.

It's not for new users at all.  The most useful application of -S and
-G is in log.  The translation from a log -G to a diffcore -G is not
obvious at all, and warrants an explanation.

Oh, and for the user.  No user is going to type out `man gitdiffcore`
out of the blue: she's most probably led there from log, and we're
connecting the dots for her.

> While the switches are called -S and -G, I do not think it is helpful
> to name the two pickaxe options as "the S kind" and "the G kind".

How do you describe something precisely without loss of meaning?  You
stop abstracting unnecessarily.  Read the sources: you will literally
see DIFF_PICKAXE_KIND_G there.

>> +
>> +The S kind detects filepairs whose "result" side and "origin" side
>> +have different number of occurrences of specified string.  While
>> +rename detection works as usual, 'git log -S' cannot omit commits
>
> The "cannot omit" feels like a confusing double-negative.  How about
> "includes" instead?

Intended.  Omission is expected.

> Is it worth mentioning that something in the documentation is "worth
> mentioning"?

You don't have to nitpick style.  We can allow this much creative
freedom in documentation.

>> +in effect, diffcore-pickaxe leaves only such filepairs that touch the
>> +specified string in its output.  When in effect, diffcore-pickaxe
>> +leaves all filepairs intact if there is such a filepair, or makes the
>> +output empty otherwise.  The latter behavior is designed to make
>> +reviewing of the changes in the context of the whole changeset easier.
>
> I find this description (from the original code, not from this commit)
> somewhat confusing.  It is written from the implementer's POV.

I explained the entire -S and -G thing in terms of filepairs (and yes,
that's implementation detail).  Why would I want to explain this in
any other terms?

> Does
> this seem clearer to you?
> [...]

>From diff-options.txt (the more end-user side):

When -S or -G finds a change, show all the changes in that changeset,
not just the files that contain the change in <string>.

Not clear enough?