Re: incorrect range-diff output?

[Thomas Gummerer <t.gummerer@gmail.com>]

On 04/11, Duy Nguyen wrote:
> Try
> 
>     git range-diff from...to
> 
> with those two branches from https://gitlab.com/pclouds/git.git. The
> interesting part is this
> 
>       diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
>       --- a/Documentation/gitcli.txt
>     @@ -120,10 +111,11 @@
>         * linkgit:git-commit[1] to advance the current branch.
>       
>      -  * linkgit:git-reset[1] and linkgit:git-checkout[1] (with
>     -+  * linkgit:git-reset[1] and linkgit:git-restore[1] (with
>     -     pathname parameters) to undo changes.
>     +-    pathname parameters) to undo changes.
>     ++  * linkgit:git-restore[1] to undo changes.
>       
>         * linkgit:git-merge[1] to merge between local branches.
>     + 
> 
> This particular hunk comes from giteveryday.txt, not gitcli.txt. And
> the b/Documentation/gitcli.txt line is also missing.

I think the output here is technically correct, even though it is very
misleading.  range-diff doesn't currently show the filenames of the
diff that changed, which makes this a bit hard to read.  To understand
what's happening here, let's have a look at the parts of the commits
where this happens:

In 66370c3f63 we have:

    diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
    index 837707a8fd..c38bc54439 100644
    --- a/Documentation/git-revert.txt
    +++ b/Documentation/git-revert.txt
    @@ -26,8 +26,8 @@ effect of some earlier commits (often only a faulty one).  If you want to
     throw away all uncommitted changes in your working directory, you
     should see linkgit:git-reset[1], particularly the `--hard` option.  If
     you want to extract specific files as they were in another commit, you
    -should see linkgit:git-checkout[1], specifically the `git checkout
    -<commit> -- <filename>` syntax.  Take care with these alternatives as
    +should see linkgit:git-restore[1], specifically the `--source`
    +option  Take care with these alternatives as
     both will discard uncommitted changes in your working directory.
 
     OPTIONS
    diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
    index 592e06d839..e2a9674297 100644
    --- a/Documentation/gitcli.txt
    +++ b/Documentation/gitcli.txt

While in 183c6c9390 we have:

    diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
    index 018ecf49d3..9aadc36881 100644
    --- a/Documentation/git-revert.txt
    +++ b/Documentation/git-revert.txt
    @@ -26,8 +26,8 @@ effect of some earlier commits (often only a faulty one).  If you want to
     throw away all uncommitted changes in your working directory, you
     should see linkgit:git-reset[1], particularly the `--hard` option.  If
     you want to extract specific files as they were in another commit, you
    -should see linkgit:git-checkout[1], specifically the `git checkout
    -<commit> -- <filename>` syntax.  Take care with these alternatives as
    +should see linkgit:git-restore[1], specifically the `--source`
    +option. Take care with these alternatives as
     both will discard uncommitted changes in your working directory.
     
     See "Reset, restore and revert" in linkgit:git[1] for the differences
    diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
    index 592e06d839..e2a9674297 100644
    --- a/Documentation/gitcli.txt
    +++ b/Documentation/gitcli.txt


So adding a bit more context to the bits you posted above we see:

    @@ -92,10 +83,10 @@
     -should see linkgit:git-checkout[1], specifically the `git checkout
     -<commit> -- <filename>` syntax.  Take care with these alternatives as
     +should see linkgit:git-restore[1], specifically the `--source`
    -+option  Take care with these alternatives as
    ++option. Take care with these alternatives as
      both will discard uncommitted changes in your working directory.
      
    - OPTIONS
    + See "Reset, restore and revert" in linkgit:git[1] for the differences
     
      diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
      --- a/Documentation/gitcli.txt
    @@ -120,10 +111,11 @@
        * linkgit:git-commit[1] to advance the current branch.
      
     -  * linkgit:git-reset[1] and linkgit:git-checkout[1] (with
    -+  * linkgit:git-reset[1] and linkgit:git-restore[1] (with
    -     pathname parameters) to undo changes.
    +-    pathname parameters) to undo changes.
    ++  * linkgit:git-restore[1] to undo changes.
      
        * linkgit:git-merge[1] to merge between local branches.
    + 
     @@
      ------------
      $ git switch -c alsa-audio <1>

So the Documentation/gitcli.txt bit is actually context on the first
diff, and everything after '@@ -120,10 +111,11 @@' is completely
unrelated to that.

I'm not sure what the right solution for this is.  I think one thing
I'd like range-diff to do is to add the filename, or some context
(e.g. is this part of the commit message etc.) to the @@ line (not
sure what that is called?).

Another thing that would help in this case would be to just remove the
diff header if it is only in the context of a diff, but not actually
changed.  We can't just remove it outright, as it could be useful in
some cases, e.g. when a file has just been re-named.

So all that said, while this is technically correct, I think it's
misleading enough that we should try to fix it.  Maybe I can find some
time over the weekend to tackle this, if nobody else gets to it first.

> I'm in the middle of some other things (and don't know range-diff that
> well) so I'm just dropping a bug report here.
> 
> I've tried all 'master', 'next' and 'pu'. Same result. Tried
> --no-pager too in case the pager ate something up.
> --
> Duy