Re: [PATCH v3 17/20] range-diff: add a man page

Hi Stefan,

On Mon, 9 Jul 2018, Stefan Beller wrote:

On Mon, Jul 9, 2018 at 1:00 PM Johannes Schindelin
[off-list ref] wrote:

quoted

On Mon, 9 Jul 2018, Stefan Beller wrote:

quoted

On Tue, Jul 3, 2018 at 4:26 AM Johannes Schindelin via GitGitGadget
[off-list ref] wrote:

quoted

+'git range-diff' [--color=[<when>]] [--no-color] [<diff-options>]
+       [--dual-color] [--creation-factor=<factor>]
+       ( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
+
+DESCRIPTION
+-----------
+
+This command shows the differences between two versions of a patch
+series, or more generally, two commit ranges (ignoring merges).

Does it completely ignore merges or does it die("not supported"), how is
the user expected to cope with the accidental merge in the given range?

It ignores merges. It does not reject them. It simply ignores them and
won't talk about them as a consequence.

Could you suggest an improved way to say that?

Well that is what the patch said already; I was just dense in reading.
I just tested it, and the commit with more than one parent itself is
ignored (not showing up in the output), but the commits that are merged
in are still considered.

So a more accurate wording would be "ignoring merge commits" rather than
"ignoring merges".

Makes sense?

So giving range1 as f7761a5a065..0a5677f6f68 with

  0a5677f6f68 (Merge branch 'js/branch-diff' into pu, 2018-07-06)
  f7761a5a065 (Merge branch 'jk/fsck-gitmodules-gently' into jch, 2018-07-06)

still produces cool output and with --word-diff it is even more amazing, as it
just tells me a large part was s/branch-diff/range-diff/ :-)

I never thought about using `--word-diff` with `range-diff`.

Sadly, for me it gives rather stupid output, e.g.

4:  6927c11a311 ! 4:  ebf3fea2517 range-diff: make --dual-color the default mode
    @@ -14,6 +14,15 @@
    diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
    --- a/Documentation/git-range-diff.txt
    +++ b/Documentation/git-range-diff.txt
    {+@@+}
    {+ --------+}
    {+ [verse]+}
    {+ 'git range-diff' [--color=[<when>]] [--no-color] [<diff-options>]+}
    {+- [--dual-color] [--creation-factor=<factor>]+}
    {++ [--no-dual-color] [--creation-factor=<factor>]+}
    {+  ( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )+}
    {+ +}
    {+ DESCRIPTION+}
    @@

     OPTIONS

quoted

quoted

quoted

+Algorithm
+---------
+
+The general idea is this: we generate a cost matrix between the commits
+in both commit ranges, then solve the least-cost assignment.

Can you say more about the generation of the cost matrix?
I assume that it counts the number of lines added/deleted to make
one patch into the other patch.

I think that is correct.

*reading the patch*

Actually, no, I was wrong. For the cost matrix, the *length* of the diff
*of the diffs* is computed. Think of it as

        git diff --no-index <(git diff A^!) <(git diff B^!) | wc -l

So the matching is based only on diffs, but the output still takes
the commit messages into account. So when diffing my series to the
series that Junio applies, (merely adding his sign off,) would be a
"cost of 0" in this context, but I still have output.

Exactly.

quoted

quoted

Another spot to look at is further metadata, such as author and
author-date, which are kept the same in a rebase workflow.

I encourage you to offer that as an add-on patch series. Because what you
suggest is not necessary for my use cases, so I'd rather not spend time on
it.

Makes sense. When I stumble about this yet theoretical problem to
materialize in practice I will send a patch. In my mind this is not
another use case, but just an improved matching, with the matching that
this series provides being good enough for now.

Sure.

Ciao,
Dscho