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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 14, 2013 at 2:44 PM, Ramkumar Ramachandra
<artagnon@xxxxxxxxx> wrote:
> 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.

She may have been led here by some other help topic, too.  Maybe the
git log examples belong in the git-log documentation.

>> 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.

S and G are abstractions.

DIFF_PICKAXE_KIND_G is an implementer's distinction.

I agree this is a tricky subject.  When writing technical
documentation you must be precise and clear.  It is easy to forget
both.  It is common to forget to be either clear or precise.  It is
very difficult to be both clear and precise.

So, my suggestion is to use some meaningful names for the action of -S
and -G.  Relating these names to -S and -G in a clear way is likely to
be difficult.

>>> +
>>> +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.

This is precision at the expense of clarity.  Omission is not expected
for the user who wants to ask this question:  "Where is that commit
that added 'foo'?"  To Git she wants to ask "Show me the commit that
added or removed 'foo'."  You and I both know that Git does this in
reverse.  Git translates this question into "Show all the commits, but
hide the ones which do not add or remove 'foo'."

And so we say Git 'cannot omit' commits


>
>> 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.

I think I do.  The concepts in here are complicated enough without
loading the language with excess verbiage.  I am chronically afflicted
with this disease where I am always adding extra decorations to my
sentences.  I work hard to combat that, especially when dealing with
technical writings.  This makes it easy for me to recognize in other
technical writing.

I didn't look closely, but these unnecessary introductory clause
appeared to be the only change in this paragraph.  I do not think it
adds anything, which is why I mentioned it.

I could have been more clear and less flippant about it, though.  I'm
sorry if my manner was offensive. This is another chronic problem I
seem to have.


>>> +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?

Clarity.

What is a 'filepair' when I am getting a short-log?  Internally there
was a diff, and the diff involves pairs of files.  But it's a tedious
detail to the user which might send her off needlessly trying to
understand the meaning of the term.

But you are right; gitdiffcore.txt is about precision and
implementation.  The goal is to address the technical user who is
trying to understand these operations in general, not just for log.
'filepairs' is a useful concept to lean on.

The clearer description probably belongs in git-log.

>> 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?

Yes, it was clear enough to me in git-log.  Perhaps it is not worth
mentioning here.

Phil
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]