Re: incorrect range-diff output?

On 04/11, Duy Nguyen wrote:

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.