On Tue, May 14, 2013 at 10:12 AM, Ramkumar Ramachandra
<artagnon@xxxxxxxxx> wrote:
> The documentation of -S and -G is very sketchy. Completely rewrite the
> sections in Documentation/diff-options.txt and
> Documentation/gitdiffcore.txt.
>
> References:
> 52e9578 ([PATCH] Introducing software archaeologist's tool "pickaxe".)
> f506b8e (git log/diff: add -G<regexp> that greps in the patch text)
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx>
> ---
> I spent some time reading the code and history to figure out what -S
> and -G really do. I hope I've done justice.
>
> Documentation/diff-options.txt | 35 +++++++++++++++++++++++++++-------
> Documentation/gitdiffcore.txt | 43 +++++++++++++++++++++++-------------------
> 2 files changed, 52 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 104579d..765abc5 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -383,14 +383,35 @@ ifndef::git-format-patch[]
> that matches other criteria, nothing is selected.
>
> -S<string>::
> - Look for differences that introduce or remove an instance of
> - <string>. Note that this is different than the string simply
> - appearing in diff output; see the 'pickaxe' entry in
> - linkgit:gitdiffcore[7] for more details.
> + Look for commits where the specified string was added or
> + removed. More precisely, find commits that change the number
> + of occurrences of the specified string.
> ++
> +It is often useful when you're looking for an exact string (like a
> +function prototype), and want to know the history of that string since
> +it first came into being.
>
> -G<regex>::
It looks like you have deleted the -S and -G references here. Am I
reading this right?
> - Look for differences whose added or removed line matches
> - the given <regex>.
> + Grep through the patch text of commits for added/removed lines
> + that match <regex>. `--pickaxe-regex` is implied in this
> + mode.
> ++
> +To illustrate the difference between `-S<regex> --pickaxe-regex` and
> +`-G<regex>`, consider a commit with the following diff in the same
> +file:
> ++
> +----
> ++ return !regexec(regexp, two->ptr, 1, ®match, 0);
> +...
> +- hit = !regexec(regexp, mf2.ptr, 1, ®match, 0);
> +----
> ++
> +While `git log -G"regexec\(regexp"` will show this commit, `git log
> +-S"regexec\(regexp" --pickaxe-regex` will not (because the number of
> +occurrences of that string didn't change).
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.
> ++
> +See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more
> +information.
>
> --pickaxe-all::
> When `-S` or `-G` finds a change, show all the changes in that
> @@ -399,7 +420,7 @@ ifndef::git-format-patch[]
>
> --pickaxe-regex::
> Make the <string> not a plain string but an extended POSIX
> - regex to match.
> + regex to match. Implied when using `-G`.
> endif::git-format-patch[]
>
> -O<orderfile>::
> diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
> index 568d757..39b9c51 100644
> --- a/Documentation/gitdiffcore.txt
> +++ b/Documentation/gitdiffcore.txt
> @@ -222,25 +222,30 @@ version prefixed with '+'.
> diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
> ---------------------------------------------------------------------
>
> -This transformation is used to find filepairs that represent
> -changes that touch a specified string, and is controlled by the
> --S option and the `--pickaxe-all` option to the 'git diff-*'
> -commands.
> -
> -When diffcore-pickaxe is in use, it checks if there are
> -filepairs whose "result" side and whose "origin" side have
> -different number of specified string. Such a filepair represents
> -"the string appeared in this changeset". It also checks for the
> -opposite case that loses the specified string.
> -
> -When `--pickaxe-all` is not in effect, diffcore-pickaxe leaves
> -only such filepairs that touch the specified string in its
> -output. When `--pickaxe-all` is used, diffcore-pickaxe leaves all
> -filepairs intact if there is such a filepair, or makes the
> -output empty otherwise. The latter behaviour is designed to
> -make reviewing of the changes in the context of the whole
> -changeset easier.
> -
> +There are two kinds of pickaxe: the S kind (corresponding to 'git log
> +-S') and the G kind (corresponding to 'git log -G').
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".
> +
> +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?
> +where a the small string being looked for is moved verbatim from one
s/a the/the/
> +file to another (since the number of occurrences of that string
> +changed in each of those two filepairs). The implementation
> +essentially runs a count, and is significantly cheaper than the G
> +kind.
> +
> +The G kind detects filepairs whose patch text has an added or a
> +deleted line that matches the given regexp. This means that it can
> +detect in-file (or what rename-detection considers the same file)
> +moves. The implementation of 'git log -G' runs diff twice and greps,
> +and this can be quite expensive.
> +
> +A diffcore-pickaxe option worth mentioning: `--pickaxe-all`. When not
Is it worth mentioning that something in the documentation is "worth
mentioning"?
> +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. Does
this seem clearer to you?
Normally the pickaxe options limit the diff output to those files which
contained the changes being searched; that is, those files which
had modifications including the search string. With the --pickaxe-all
option, the diff of the entire commit will be shown including files
which did not have modifications including the search string. This
is designed to make it easier to review the changes in the context
of the whole commit.
>
> diffcore-order: For Sorting the Output Based on Filenames
> ---------------------------------------------------------
> --
> 1.8.3.rc1.61.g2cacfff
>
> --
> 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
--
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