Re: [PATCH] Bust the ghost of long-defunct diffcore-pathspec.

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 1, 2008, 23:00 UTC
Message-ID: <7vbpz7tqsb.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080901215353.30399.33432.stgit@gandelf.nowhere.earth>

Yann Dirson <ydirson@altern.org> writes:

> This concept was retired by 77882f60d9df2fd410ba7d732b01738315643c05,
> more than 2 years ago.
> ---

Signoff.

Show 15 quoted lines

> diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
> index 2bdbc3d..fc54bfb 100644
> --- a/Documentation/gitdiffcore.txt
> +++ b/Documentation/gitdiffcore.txt
> @@ -52,9 +52,8 @@ unmerged       :000000 000000 0000000... 0000000... U file6
>  The diffcore mechanism is fed a list of such comparison results
>  (each of which is called "filepair", although at this point each
>  of them talks about a single file), and transforms such a list
> -into another list.  There are currently 6 such transformations:
> +into another list.  There are currently 5 such transformations:
>
> -- diffcore-pathspec
>  - diffcore-break
>  - diffcore-rename
>  - diffcore-merge-broken

I left this in more-or-less deliberately, because this description is still true at the conceptual level (which this document is about).

But I am not opposed to your change in this hunk...

Show 12 quoted lines

> @@ -62,21 +61,22 @@ into another list.  There are currently 6 such transformations:
>  - diffcore-order
>  
>  These are applied in sequence.  The set of filepairs 'git-diff-{asterisk}'
> -commands find are used as the input to diffcore-pathspec, and
> -the output from diffcore-pathspec is used as the input to the
> +commands find are used as the input to diffcore-break, and
> +the output from diffcore-break is used as the input to the
>  next transformation.  The final result is then passed to the
>  output routine and generates either diff-raw format (see Output
>  format sections of the manual for 'git-diff-{asterisk}' commands) or
>  diff-patch format.

... and this part of the hunk is also fine.

Show 8 quoted lines

> -diffcore-pathspec: For Ignoring Files Outside Our Consideration
> ----------------------------------------------------------------
> +Pathspec filtering: For Ignoring Files Outside Our Consideration
> +----------------------------------------------------------------
>  
> -The first transformation in the chain is diffcore-pathspec, and
> +The first transformation in the chain is pathspec filtering, which
> +occurs before calling diffcore, and

... however, this is not. You changed the way how diffcore works at the conceptual level with your change, and pathspec filtering is _NOT_ part of what diffcore does anymore, in this document.

So I'd suggest to remove this subsection altogether, and instead add some sentence to describe that the list fed to diffcore machinery is already narrowed by the pathspecs, around:

    ...
     - 'git-diff-tree' compares contents of two "tree" objects;

    In all of these cases, the commands themselves compare
    corresponding paths in the two sets of files.  The result of
    comparison is passed from these commands to what is internally
    ...

Namely, say something like:

    In all of these cases, the commands themselves first limit the two
    sets of files by pathspecs (if given), and compare corresponding paths
    in the resulting sets.  The result of the comparison is passed ...

Using the description from this part...

Show 7 quoted lines

>  is controlled by giving the pathname parameters to the
> -'git-diff-{asterisk}' commands on the command line.  The pathspec is used
> -to limit the world diff operates in.  It removes the filepairs
> +'git-diff-{asterisk}' commands on the command line.  The pathspec is
> +used to limit the world diff operates in.  It removes the filepairs
>  outside the specified set of pathnames.  E.g. If the input set
>  of filepairs included:

The last two hunks to remove "Implementation note" and "extern void ..." are good.

Thanks.