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

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

[Yann Dirson]

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

 Documentation/gitdiffcore.txt |   23 +++++++++--------------
 diffcore.h                    |    1 -
 2 files changed, 9 insertions(+), 15 deletions(-)

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
@@ -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.
 
 
-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
 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:
 
@@ -88,11 +88,6 @@ but the command invocation was `git diff-files myfile`, then the
 junkfile entry would be removed from the list because only "myfile"
 is under consideration.
 
-Implementation note.  For performance reasons, 'git-diff-tree'
-uses the pathname parameters on the command line to cull set of
-filepairs it feeds the diffcore mechanism itself, and does not
-use diffcore-pathspec, but the end result is the same.
-
 
 diffcore-break: For Splitting Up "Complete Rewrites"
 ----------------------------------------------------
diff --git a/diffcore.h b/diffcore.h
index cc96c20..8ae3578 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -92,7 +92,6 @@ extern struct diff_filepair *diff_queue(struct diff_queue_struct *,
 					struct diff_filespec *);
 extern void diff_q(struct diff_queue_struct *, struct diff_filepair *);
 
-extern void diffcore_pathspec(const char **pathspec);
 extern void diffcore_break(int);
 extern void diffcore_rename(struct diff_options *);
 extern void diffcore_merge_broken(void);

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

[Junio C Hamano]

Yann Dirson <ydirson@altern.org> writes:

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

Signoff.

> 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...

> @@ -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.

> -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...

>  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.

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

[Yann Dirson]

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

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

This new version ought to address all points in the comments to my
original patch.

 Documentation/gitdiffcore.txt |   55 ++++++++++++++++-------------------------
 diffcore.h                    |    1 -
 2 files changed, 22 insertions(+), 34 deletions(-)

diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
index 2bdbc3d..e8041bc 100644
--- a/Documentation/gitdiffcore.txt
+++ b/Documentation/gitdiffcore.txt
@@ -36,11 +36,25 @@ files:
 
  - '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
-called "diffcore", in a format similar to what is output when
-the -p option is not used.  E.g.
+In all of these cases, the commands themselves first optionally limit
+the two sets of files by any pathspecs given on their command-lines,
+and compare corresponding paths in the two resulting sets of files.
+
+The pathspecs are used to limit the world diff operates in.  They remove
+the filepairs outside the specified sets of pathnames.  E.g. If the
+input set of filepairs included:
+
+------------------------------------------------
+:100644 100644 bcd1234... 0123456... M junkfile
+------------------------------------------------
+
+but the command invocation was `git diff-files myfile`, then the
+junkfile entry would be removed from the list because only "myfile"
+is under consideration.
+
+The result of comparison is passed from these commands to what is
+internally called "diffcore", in a format similar to what is output
+when the -p option is not used.  E.g.
 
 ------------------------------------------------
 in-place edit  :100644 100644 bcd1234... 0123456... M file0
@@ -52,9 +66,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
@@ -62,38 +75,14 @@ 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.
 
 
-diffcore-pathspec: For Ignoring Files Outside Our Consideration
----------------------------------------------------------------
-
-The first transformation in the chain is diffcore-pathspec, and
-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
-outside the specified set of pathnames.  E.g. If the input set
-of filepairs included:
-
-------------------------------------------------
-:100644 100644 bcd1234... 0123456... M junkfile
-------------------------------------------------
-
-but the command invocation was `git diff-files myfile`, then the
-junkfile entry would be removed from the list because only "myfile"
-is under consideration.
-
-Implementation note.  For performance reasons, 'git-diff-tree'
-uses the pathname parameters on the command line to cull set of
-filepairs it feeds the diffcore mechanism itself, and does not
-use diffcore-pathspec, but the end result is the same.
-
-
 diffcore-break: For Splitting Up "Complete Rewrites"
 ----------------------------------------------------
 
diff --git a/diffcore.h b/diffcore.h
index cc96c20..8ae3578 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -92,7 +92,6 @@ extern struct diff_filepair *diff_queue(struct diff_queue_struct *,
 					struct diff_filespec *);
 extern void diff_q(struct diff_queue_struct *, struct diff_filepair *);
 
-extern void diffcore_pathspec(const char **pathspec);
 extern void diffcore_break(int);
 extern void diffcore_rename(struct diff_options *);
 extern void diffcore_merge_broken(void);