I (Julian Foad) wrote:
> The attached brute-force patch seems to fix many of the cases where 
> changelist 
> filtering was missing or wrong, but I don't much like it and haven't 
> properly tested it.

So here, attached, is an implementation of the post-filtering approach. This is 
implemented using a common filter for repos-WC and WC-WC diffs.

>>  Personally I would wish we could just do the changelist filtering in the
>>  output layer, instead of in all the diff drivers separately... But the
>>  current code doesn't properly guarantee access to the local working copy
>>  paths to do that kind of processing there. 
>> 
>>  In many cases it prefers to just pass something url like. (Not necessary the
>>  proper url)
> 
> If filtering at the output layer is easier that's totally fine with me, 
> anything as long as it's simple and consistent.

What do you think of the attached implementation?

I gave it a try by hand. I can't claim to have tested it thoroughly.

- Julian
Implement changelist filtering in WC diffs by post-filtering.

This should be easier to get right and to maintain than embedding changelist
filtering in both of the diff implementations. (The previous implementation
had several errors and omissions.) On the other hand this approach is less
efficient.

### TODO: Needs tests.

* subversion/libsvn_wc/diff.h
  (svn_wc__diff_local_only_file,
   svn_wc__diff_local_only_dir,
   svn_wc__diff_base_working_diff): Remove changelist filtering.
  (svn_wc__changelist_filter_tree_processor_create): New.

* subversion/libsvn_wc/diff_editor.c
  (edit_baton_t ,
   make_edit_baton,
   svn_wc__diff_base_working_diff,
   walk_local_nodes_diff,
   svn_wc__diff_local_only_file,
   svn_wc__diff_local_only_dir,
   handle_local_only): Remove changelist filtering.
  (close_file): Make sure we don't expect to see a valid checksum if we're
    skipping the node, as the sender may not have sent a diff at all.
  (svn_wc__get_diff_editor): Wrap the diff processor in a changelist
    filtering diff processor.
  (filter_tree_baton_t,
   filter_*,
   svn_wc__changelist_filter_tree_processor_create): New.

* subversion/libsvn_wc/diff_local.c
  (diff_baton,
   diff_status_callback): Remove changelist filtering.
  (svn_wc__diff7): Wrap the diff processor in a changelist
--This line, and those below, will be ignored--

Index: subversion/libsvn_wc/diff_editor.c
===================================================================
--- subversion/libsvn_wc/diff_editor.c	(revision 1621949)
+++ subversion/libsvn_wc/diff_editor.c	(working copy)
@@ -112,15 +112,12 @@ struct edit_baton_t
   /* Should this diff ignore node ancestry? */
   svn_boolean_t ignore_ancestry;
 
   /* Possibly diff repos against text-bases instead of working files. */
   svn_boolean_t diff_pristine;
 
-  /* Hash whose keys are const char * changelist names. */
-  apr_hash_t *changelist_hash;
-
   /* Cancel function/baton */
   svn_cancel_func_t cancel_func;
   void *cancel_baton;
 
   apr_pool_t *pool;
 };
@@ -252,36 +249,29 @@ make_edit_baton(struct edit_baton_t **ed
                 const char *target,
                 const svn_diff_tree_processor_t *diff_processor,
                 svn_depth_t depth,
                 svn_boolean_t ignore_ancestry,
                 svn_boolean_t use_text_base,
                 svn_boolean_t reverse_order,
-                const apr_array_header_t *changelist_filter,
                 svn_cancel_func_t cancel_func,
                 void *cancel_baton,
                 apr_pool_t *pool)
 {
-  apr_hash_t *changelist_hash = NULL;
   struct edit_baton_t *eb;
 
   SVN_ERR_ASSERT(svn_dirent_is_absolute(anchor_abspath));
 
-  if (changelist_filter && changelist_filter->nelts)
-    SVN_ERR(svn_hash_from_cstring_keys(&changelist_hash, changelist_filter,
-                                       pool));
-
   eb = apr_pcalloc(pool, sizeof(*eb));
   eb->db = db;
   eb->anchor_abspath = apr_pstrdup(pool, anchor_abspath);
   eb->target = apr_pstrdup(pool, target);
   eb->processor = diff_processor;
   eb->depth = depth;
   eb->ignore_ancestry = ignore_ancestry;
   eb->local_before_remote = reverse_order;
   eb->diff_pristine = use_text_base;
-  eb->changelist_hash = changelist_hash;
   eb->cancel_func = cancel_func;
   eb->cancel_baton = cancel_baton;
   eb->pool = pool;
 
   *edit_baton = eb;
   return SVN_NO_ERROR;
@@ -389,13 +379,12 @@ maybe_done(struct dir_baton_t *db)
 
 svn_error_t *
 svn_wc__diff_base_working_diff(svn_wc__db_t *db,
                                const char *local_abspath,
                                const char *relpath,
                                svn_revnum_t revision,
-                               apr_hash_t *changelist_hash,
                                const svn_diff_tree_processor_t *processor,
                                void *processor_dir_baton,
                                svn_boolean_t diff_pristine,
                                svn_cancel_func_t cancel_func,
                                void *cancel_baton,
                                apr_pool_t *scratch_pool)
@@ -430,18 +419,12 @@ svn_wc__diff_base_working_diff(svn_wc__d
   checksum = working_checksum;
 
   assert(status == svn_wc__db_status_normal
          || status == svn_wc__db_status_added
          || (status == svn_wc__db_status_deleted && diff_pristine));
 
-  /* If the item is not a member of a specified changelist (and there are
-     some specified changelists), skip it. */
-  if (changelist_hash && !svn_hash_gets(changelist_hash, changelist))
-    return SVN_NO_ERROR;
-
-
   if (status != svn_wc__db_status_normal)
     {
       SVN_ERR(svn_wc__db_base_get_info(&base_status, NULL, &db_revision,
                                        NULL, NULL, NULL, NULL, NULL, NULL,
                                        NULL, &checksum, NULL, NULL, &had_props,
                                        NULL, NULL,
@@ -765,23 +748,21 @@ walk_local_nodes_diff(struct edit_baton_
           if (eb->local_before_remote && local_only)
             {
               if (info->kind == svn_node_file && diff_files)
                 SVN_ERR(svn_wc__diff_local_only_file(db, child_abspath,
                                                      child_relpath,
                                                      eb->processor, dir_baton,
-                                                     eb->changelist_hash,
                                                      eb->diff_pristine,
                                                      eb->cancel_func,
                                                      eb->cancel_baton,
                                                      iterpool));
               else if (info->kind == svn_node_dir && diff_dirs)
                 SVN_ERR(svn_wc__diff_local_only_dir(db, child_abspath,
                                                     child_relpath,
                                                     depth_below_here,
                                                     eb->processor, dir_baton,
-                                                    eb->changelist_hash,
                                                     eb->diff_pristine,
                                                     eb->cancel_func,
                                                     eb->cancel_baton,
                                                     iterpool));
             }
 
@@ -811,13 +792,12 @@ walk_local_nodes_diff(struct edit_baton_
                       || !eb->diff_pristine)
                     {
                       SVN_ERR(svn_wc__diff_base_working_diff(
                                                 db, child_abspath,
                                                 child_relpath,
                                                 eb->revnum,
-                                                eb->changelist_hash,
                                                 eb->processor, dir_baton,
                                                 eb->diff_pristine,
                                                 eb->cancel_func,
                                                 eb->cancel_baton,
                                                 scratch_pool));
                     }
@@ -834,22 +814,20 @@ walk_local_nodes_diff(struct edit_baton_
           if (!eb->local_before_remote && local_only)
             {
               if (info->kind == svn_node_file && diff_files)
                 SVN_ERR(svn_wc__diff_local_only_file(db, child_abspath,
                                                      child_relpath,
                                                      eb->processor, dir_baton,
-                                                     eb->changelist_hash,
                                                      eb->diff_pristine,
                                                      eb->cancel_func,
                                                      eb->cancel_baton,
                                                      iterpool));
               else if (info->kind == svn_node_dir && diff_dirs)
                 SVN_ERR(svn_wc__diff_local_only_dir(db, child_abspath,
                                                      child_relpath, depth_below_here,
                                                      eb->processor, dir_baton,
-                                                     eb->changelist_hash,
                                                      eb->diff_pristine,
                                                      eb->cancel_func,
                                                      eb->cancel_baton,
                                                      iterpool));
             }
         }
@@ -861,13 +839,12 @@ walk_local_nodes_diff(struct edit_baton_
     /* Check for local property mods on this directory, if we haven't
      already reported them and we aren't changelist-filted.
      ### it should be noted that we do not currently allow directories
      ### to be part of changelists, so if a changelist is provided, the
      ### changelist check will always fail. */
   if (! skip
-      && ! eb->changelist_hash
       && ! in_anchor_not_target
       && props_mod)
     {
       apr_array_header_t *propchanges;
       apr_hash_t *left_props;
       apr_hash_t *right_props;
@@ -904,13 +881,12 @@ walk_local_nodes_diff(struct edit_baton_
 svn_error_t *
 svn_wc__diff_local_only_file(svn_wc__db_t *db,
                              const char *local_abspath,
                              const char *relpath,
                              const svn_diff_tree_processor_t *processor,
                              void *processor_parent_baton,
-                             apr_hash_t *changelist_hash,
                              svn_boolean_t diff_pristine,
                              svn_cancel_func_t cancel_func,
                              void *cancel_baton,
                              apr_pool_t *scratch_pool)
 {
   svn_diff_source_t *right_src;
@@ -944,16 +920,12 @@ svn_wc__diff_local_only_file(svn_wc__db_
   assert(kind == svn_node_file
          && (status == svn_wc__db_status_normal
              || status == svn_wc__db_status_added
              || (status == svn_wc__db_status_deleted && diff_pristine)));
 
 
-  if (changelist && changelist_hash
-      && !svn_hash_gets(changelist_hash, changelist))
-    return SVN_NO_ERROR;
-
   if (status == svn_wc__db_status_deleted)
     {
       assert(diff_pristine);
 
       SVN_ERR(svn_wc__db_read_pristine_info(&status, &kind, NULL, NULL, NULL,
                                             NULL, &checksum, NULL, &had_props,
@@ -1050,13 +1022,12 @@ svn_error_t *
 svn_wc__diff_local_only_dir(svn_wc__db_t *db,
                             const char *local_abspath,
                             const char *relpath,
                             svn_depth_t depth,
                             const svn_diff_tree_processor_t *processor,
                             void *processor_parent_baton,
-                            apr_hash_t *changelist_hash,
                             svn_boolean_t diff_pristine,
                             svn_cancel_func_t cancel_func,
                             void *cancel_baton,
                             apr_pool_t *scratch_pool)
 {
   svn_wc__db_status_t status;
@@ -1124,12 +1095,13 @@ svn_wc__diff_local_only_dir(svn_wc__db_t
                                 NULL,
                                 right_src,
                                 copyfrom_src,
                                 processor_parent_baton,
                                 processor,
                                 scratch_pool, iterpool));
+  /* ### skip_children is not used */
 
   SVN_ERR(svn_wc__db_read_children_info(&nodes, &conflicts, db, local_abspath,
                                         FALSE /* base_tree_only */,
                                         scratch_pool, iterpool));
 
   if (depth_below_here == svn_depth_immediates)
@@ -1169,25 +1141,23 @@ svn_wc__diff_local_only_dir(svn_wc__db_t
         {
         case svn_node_file:
         case svn_node_symlink:
           SVN_ERR(svn_wc__diff_local_only_file(db, child_abspath,
                                                child_relpath,
                                                processor, pdb,
-                                               changelist_hash,
                                                diff_pristine,
                                                cancel_func, cancel_baton,
                                                scratch_pool));
           break;
 
         case svn_node_dir:
           if (depth > svn_depth_files || depth == svn_depth_unknown)
             {
               SVN_ERR(svn_wc__diff_local_only_dir(db, child_abspath,
                                                   child_relpath, depth_below_here,
                                                   processor, pdb,
-                                                  changelist_hash,
                                                   diff_pristine,
                                                   cancel_func, cancel_baton,
                                                   iterpool));
             }
           break;
 
@@ -1279,24 +1249,22 @@ handle_local_only(struct dir_baton_t *pb
       SVN_ERR(svn_wc__diff_local_only_dir(
                       eb->db,
                       svn_dirent_join(pb->local_abspath, name, scratch_pool),
                       svn_relpath_join(pb->relpath, name, scratch_pool),
                       repos_delete ? svn_depth_infinity : depth,
                       eb->processor, pb->pdb,
-                      eb->changelist_hash,
                       eb->diff_pristine,
                       eb->cancel_func, eb->cancel_baton,
                       scratch_pool));
     }
   else
     SVN_ERR(svn_wc__diff_local_only_file(
                       eb->db,
                       svn_dirent_join(pb->local_abspath, name, scratch_pool),
                       svn_relpath_join(pb->relpath, name, scratch_pool),
                       eb->processor, pb->pdb,
-                      eb->changelist_hash,
                       eb->diff_pristine,
                       eb->cancel_func, eb->cancel_baton,
                       scratch_pool));
 
   return SVN_NO_ERROR;
 }
@@ -2065,13 +2033,20 @@ close_file(void *file_baton,
   apr_pool_t *scratch_pool = fb->pool;
 
   /* The repository information; constructed from BASE + Changes */
   const char *repos_file;
   apr_hash_t *repos_props;
 
-  if (!fb->skip && expected_md5_digest != NULL)
+  if (fb->skip)
+    {
+      svn_pool_destroy(fb->pool); /* destroys scratch_pool and fb */
+      SVN_ERR(maybe_done(pb));
+      return SVN_NO_ERROR;
+    }
+
+  if (expected_md5_digest != NULL)
     {
       svn_checksum_t *expected_checksum;
       const svn_checksum_t *result_checksum;
 
       if (fb->temp_file_path)
         result_checksum = svn_checksum__from_digest_md5(fb->result_digest,
@@ -2120,17 +2095,13 @@ close_file(void *file_baton,
                                              eb->db, eb->anchor_abspath,
                                              fb->base_checksum,
                                              scratch_pool, scratch_pool));
       }
   }
 
-  if (fb->skip)
-    {
-      /* Diff processor requested skipping information */
-    }
-  else if (fb->repos_only)
+  if (fb->repos_only)
     {
       SVN_ERR(eb->processor->file_deleted(fb->relpath,
                                           fb->left_src,
                                           fb->temp_file_path,
                                           repos_props,
                                           fb->pfb,
@@ -2304,18 +2275,30 @@ svn_wc__get_diff_editor(const svn_delta_
   struct svn_wc__shim_fetch_baton_t *sfb;
   svn_delta_shim_callbacks_t *shim_callbacks =
                                 svn_delta_shim_callbacks_default(result_pool);
 
   SVN_ERR_ASSERT(svn_dirent_is_absolute(anchor_abspath));
 
+  /* Apply changelist filtering to the output */
+  if (changelist_filter && changelist_filter->nelts)
+    {
+      apr_hash_t *changelist_hash;
+
+      SVN_ERR(svn_hash_from_cstring_keys(&changelist_hash, changelist_filter,
+                                         result_pool));
+      diff_processor = svn_wc__changelist_filter_tree_processor_create(
+                         diff_processor, wc_ctx, anchor_abspath,
+                         changelist_hash, result_pool);
+    }
+
   SVN_ERR(make_edit_baton(&eb,
                           wc_ctx->db,
                           anchor_abspath, target,
                           diff_processor,
                           depth, ignore_ancestry,
-                          use_text_base, reverse_order, changelist_filter,
+                          use_text_base, reverse_order,
                           cancel_func, cancel_baton,
                           result_pool));
 
   tree_editor = svn_delta_default_editor(eb->pool);
 
   tree_editor->set_target_revision = set_target_revision;
@@ -2775,6 +2758,349 @@ svn_wc__wrap_diff_callbacks(const svn_di
   processor->file_changed  = wrap_file_changed;
   /*processor->file_closed   = wrap_file_closed*/; /* Not needed */
 
   *diff_processor = processor;
   return SVN_NO_ERROR;
 }
+
+/* =====================================================================
+ * A tree processor filter that filters by changelist membership
+ * =====================================================================
+ *
+ * The current implementation queries the WC for the changelist of each
+ * file as it comes through, and sets the 'skip' flag for a non-matching
+ * file.
+ *
+ * (It doesn't set the 'skip' flag for a directory, as we need to receive
+ * the changed/added/deleted/closed call to know when it is closed, in
+ * order to preserve the strict open-close semantics for the wrapped tree
+ * processor.)
+ *
+ * It passes on the opening and closing of every directory, even if there
+ * are no file changes to be passed on inside that directory.
+ */
+
+typedef struct filter_tree_baton_t
+{
+  const svn_diff_tree_processor_t *processor;
+  svn_wc_context_t *wc_ctx;
+  /* WC path of the root of the diff (where relpath = "") */
+  const char *root_local_abspath;
+  /* Hash whose keys are const char * changelist names. */
+  apr_hash_t *changelist_hash;
+} filter_tree_baton_t;
+
+static svn_error_t *
+filter_dir_opened(void **new_dir_baton,
+                  svn_boolean_t *skip,
+                  svn_boolean_t *skip_children,
+                  const char *relpath,
+                  const svn_diff_source_t *left_source,
+                  const svn_diff_source_t *right_source,
+                  const svn_diff_source_t *copyfrom_source,
+                  void *parent_dir_baton,
+                  const svn_diff_tree_processor_t *processor,
+                  apr_pool_t *result_pool,
+                  apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+
+  SVN_ERR(fb->processor->dir_opened(new_dir_baton, skip, skip_children,
+                                    relpath,
+                                    left_source, right_source,
+                                    copyfrom_source,
+                                    parent_dir_baton,
+                                    fb->processor,
+                                    result_pool, scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+filter_dir_added(const char *relpath,
+                 const svn_diff_source_t *copyfrom_source,
+                 const svn_diff_source_t *right_source,
+                 /*const*/ apr_hash_t *copyfrom_props,
+                 /*const*/ apr_hash_t *right_props,
+                 void *dir_baton,
+                 const svn_diff_tree_processor_t *processor,
+                 apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+
+  SVN_ERR(fb->processor->dir_closed(relpath,
+                                    NULL,
+                                    right_source,
+                                    dir_baton,
+                                    fb->processor,
+                                    scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+filter_dir_deleted(const char *relpath,
+                   const svn_diff_source_t *left_source,
+                   /*const*/ apr_hash_t *left_props,
+                   void *dir_baton,
+                   const svn_diff_tree_processor_t *processor,
+                   apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+
+  SVN_ERR(fb->processor->dir_closed(relpath,
+                                    left_source,
+                                    NULL,
+                                    dir_baton,
+                                    fb->processor,
+                                    scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+filter_dir_changed(const char *relpath,
+                   const svn_diff_source_t *left_source,
+                   const svn_diff_source_t *right_source,
+                   /*const*/ apr_hash_t *left_props,
+                   /*const*/ apr_hash_t *right_props,
+                   const apr_array_header_t *prop_changes,
+                   void *dir_baton,
+                   const struct svn_diff_tree_processor_t *processor,
+                   apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+
+  SVN_ERR(fb->processor->dir_closed(relpath,
+                                    left_source,
+                                    right_source,
+                                    dir_baton,
+                                    fb->processor,
+                                    scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+filter_dir_closed(const char *relpath,
+                  const svn_diff_source_t *left_source,
+                  const svn_diff_source_t *right_source,
+                  void *dir_baton,
+                  const svn_diff_tree_processor_t *processor,
+                  apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+
+  SVN_ERR(fb->processor->dir_closed(relpath,
+                                    left_source,
+                                    right_source,
+                                    dir_baton,
+                                    fb->processor,
+                                    scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+filter_file_opened(void **new_file_baton,
+                   svn_boolean_t *skip,
+                   const char *relpath,
+                   const svn_diff_source_t *left_source,
+                   const svn_diff_source_t *right_source,
+                   const svn_diff_source_t *copyfrom_source,
+                   void *dir_baton,
+                   const svn_diff_tree_processor_t *processor,
+                   apr_pool_t *result_pool,
+                   apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+  const char *local_abspath
+    = svn_dirent_join(fb->root_local_abspath, relpath, scratch_pool);
+  const char *changelist;
+  svn_error_t *err;
+
+  /* Skip if not a member of a given changelist*/
+  err = svn_wc__db_read_info(NULL, NULL, NULL, NULL, NULL, NULL,
+                             NULL, NULL, NULL, NULL, NULL, NULL,
+                             NULL, NULL, NULL, NULL, NULL, NULL,
+                             NULL, &changelist, NULL, NULL, NULL, NULL,
+                             NULL, NULL, NULL,
+                             fb->wc_ctx->db, local_abspath,
+                             scratch_pool, scratch_pool);
+  if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
+    {
+      svn_error_clear(err);
+      changelist = NULL;
+    }
+  else
+    SVN_ERR(err);
+  if (fb->changelist_hash
+      && (!changelist || !svn_hash_gets(fb->changelist_hash, changelist)))
+    {
+      *skip = TRUE;
+      SVN_DBG(("filter: skipping '%s'", relpath));
+      return SVN_NO_ERROR;
+    }
+
+  SVN_ERR(fb->processor->file_opened(new_file_baton,
+                                     skip,
+                                     relpath,
+                                     left_source,
+                                     right_source,
+                                     copyfrom_source,
+                                     dir_baton,
+                                     fb->processor,
+                                     result_pool,
+                                     scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+filter_file_added(const char *relpath,
+                  const svn_diff_source_t *copyfrom_source,
+                  const svn_diff_source_t *right_source,
+                  const char *copyfrom_file,
+                  const char *right_file,
+                  /*const*/ apr_hash_t *copyfrom_props,
+                  /*const*/ apr_hash_t *right_props,
+                  void *file_baton,
+                  const svn_diff_tree_processor_t *processor,
+                  apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+
+  SVN_ERR(fb->processor->file_added(relpath,
+                                    copyfrom_source,
+                                    right_source,
+                                    copyfrom_file,
+                                    right_file,
+                                    copyfrom_props,
+                                    right_props,
+                                    file_baton,
+                                    fb->processor,
+                                    scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+filter_file_deleted(const char *relpath,
+                    const svn_diff_source_t *left_source,
+                    const char *left_file,
+                    /*const*/ apr_hash_t *left_props,
+                    void *file_baton,
+                    const svn_diff_tree_processor_t *processor,
+                    apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+
+  SVN_ERR(fb->processor->file_deleted(relpath,
+                                      left_source,
+                                      left_file,
+                                      left_props,
+                                      file_baton,
+                                      fb->processor,
+                                      scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+filter_file_changed(const char *relpath,
+                    const svn_diff_source_t *left_source,
+                    const svn_diff_source_t *right_source,
+                    const char *left_file,
+                    const char *right_file,
+                    /*const*/ apr_hash_t *left_props,
+                    /*const*/ apr_hash_t *right_props,
+                    svn_boolean_t file_modified,
+                    const apr_array_header_t *prop_changes,
+                    void *file_baton,
+                    const svn_diff_tree_processor_t *processor,
+                    apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+
+  SVN_ERR(fb->processor->file_changed(relpath,
+                                      left_source,
+                                      right_source,
+                                      left_file,
+                                      right_file,
+                                      left_props,
+                                      right_props,
+                                      file_modified,
+                                      prop_changes,
+                                      file_baton,
+                                      fb->processor,
+                                      scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+filter_file_closed(const char *relpath,
+                   const svn_diff_source_t *left_source,
+                   const svn_diff_source_t *right_source,
+                   void *file_baton,
+                   const svn_diff_tree_processor_t *processor,
+                   apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+
+  SVN_ERR(fb->processor->file_closed(relpath,
+                                     left_source,
+                                     right_source,
+                                     file_baton,
+                                     fb->processor,
+                                     scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+filter_node_absent(const char *relpath,
+                   void *dir_baton,
+                   const svn_diff_tree_processor_t *processor,
+                   apr_pool_t *scratch_pool)
+{
+  struct filter_tree_baton_t *fb = processor->baton;
+
+  SVN_ERR(fb->processor->node_absent(relpath,
+                                     dir_baton,
+                                     fb->processor,
+                                     scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+const svn_diff_tree_processor_t *
+svn_wc__changelist_filter_tree_processor_create(
+                                const svn_diff_tree_processor_t *processor,
+                                svn_wc_context_t *wc_ctx,
+                                const char *root_local_abspath,
+                                apr_hash_t *changelist_hash,
+                                apr_pool_t *result_pool)
+{
+  struct filter_tree_baton_t *fb;
+  svn_diff_tree_processor_t *filter;
+
+  if (! changelist_hash)
+    return processor;
+
+  fb = apr_pcalloc(result_pool, sizeof(*fb));
+  fb->processor = processor;
+  fb->wc_ctx = wc_ctx;
+  fb->root_local_abspath = root_local_abspath;
+  fb->changelist_hash = changelist_hash;
+
+  filter = svn_diff__tree_processor_create(fb, result_pool);
+  filter->dir_opened   = filter_dir_opened;
+  filter->dir_added    = filter_dir_added;
+  filter->dir_deleted  = filter_dir_deleted;
+  filter->dir_changed  = filter_dir_changed;
+  filter->dir_closed   = filter_dir_closed;
+
+  filter->file_opened   = filter_file_opened;
+  filter->file_added    = filter_file_added;
+  filter->file_deleted  = filter_file_deleted;
+  filter->file_changed  = filter_file_changed;
+  filter->file_closed   = filter_file_closed;
+
+  filter->node_absent   = filter_node_absent;
+
+  return filter;
+}
+
Index: subversion/libsvn_wc/diff.h
===================================================================
--- subversion/libsvn_wc/diff.h	(revision 1621949)
+++ subversion/libsvn_wc/diff.h	(working copy)
@@ -54,13 +54,12 @@ extern "C" {
 svn_error_t *
 svn_wc__diff_local_only_file(svn_wc__db_t *db,
                              const char *local_abspath,
                              const char *relpath,
                              const svn_diff_tree_processor_t *processor,
                              void *processor_parent_baton,
-                             apr_hash_t *changelist_hash,
                              svn_boolean_t diff_pristine,
                              svn_cancel_func_t cancel_func,
                              void *cancel_baton,
                              apr_pool_t *scratch_pool);
 
 /* A function to diff locally added and locally copied directories.
@@ -81,13 +80,12 @@ svn_error_t *
 svn_wc__diff_local_only_dir(svn_wc__db_t *db,
                             const char *local_abspath,
                             const char *relpath,
                             svn_depth_t depth,
                             const svn_diff_tree_processor_t *processor,
                             void *processor_parent_baton,
-                            apr_hash_t *changelist_hash,
                             svn_boolean_t diff_pristine,
                             svn_cancel_func_t cancel_func,
                             void *cancel_baton,
                             apr_pool_t *scratch_pool);
 
 /* Reports the BASE-file LOCAL_ABSPATH as deleted to PROCESSOR with relpath
@@ -129,20 +127,45 @@ svn_wc__diff_base_only_dir(svn_wc__db_t
  */
 svn_error_t *
 svn_wc__diff_base_working_diff(svn_wc__db_t *db,
                                const char *local_abspath,
                                const char *relpath,
                                svn_revnum_t revision,
-                               apr_hash_t *changelist_hash,
                                const svn_diff_tree_processor_t *processor,
                                void *processor_dir_baton,
                                svn_boolean_t diff_pristine,
                                svn_cancel_func_t cancel_func,
                                void *cancel_baton,
                                apr_pool_t *scratch_pool);
 
+/* Return a tree processor filter that filters by changelist membership.
+ *
+ * This filter only passes on the changes for a file if the file's path
+ * (in the WC) is assigned to one of the changelists in @a changelist_hash.
+ * It also passes on the opening and closing of each directory that contains
+ * such a change, and possibly also of other directories, but not addition
+ * or deletion or changes to a directory.
+ *
+ * If @a changelist_hash is null then no filtering is performed and the
+ * returned diff processor is driven exactly like the input @a processor.
+ *
+ * @a wc_ctx is the WC context and @a root_local_abspath is the WC path of
+ * the root of the diff (for which relpath = "" in the diff processor).
+ *
+ * Allocate the returned diff processor in @a result_pool, or if no
+ * filtering is required then the input pointer @a processor itself may be
+ * returned.
+ */
+const svn_diff_tree_processor_t *
+svn_wc__changelist_filter_tree_processor_create(
+                                const svn_diff_tree_processor_t *processor,
+                                svn_wc_context_t *wc_ctx,
+                                const char *root_local_abspath,
+                                apr_hash_t *changelist_hash,
+                                apr_pool_t *result_pool);
+
 
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
 
 #endif /* SVN_LIBSVN_WC_DIFF_H */
Index: subversion/libsvn_wc/diff_local.c
===================================================================
--- subversion/libsvn_wc/diff_local.c	(revision 1621949)
+++ subversion/libsvn_wc/diff_local.c	(working copy)
@@ -86,15 +86,12 @@ struct diff_baton
 
   const svn_diff_tree_processor_t *processor;
 
   /* Should this diff ignore node ancestry? */
   svn_boolean_t ignore_ancestry;
 
-  /* Hash whose keys are const char * changelist names. */
-  apr_hash_t *changelist_hash;
-
   /* Cancel function/baton */
   svn_cancel_func_t cancel_func;
   void *cancel_baton;
 
   apr_pool_t *pool;
 };
@@ -246,17 +243,12 @@ diff_status_callback(void *baton,
   SVN_ERR(ensure_state(eb, svn_dirent_dirname(local_abspath, scratch_pool),
                        FALSE, scratch_pool));
 
   if (eb->cur && eb->cur->skip_children)
     return SVN_NO_ERROR;
 
-  if (eb->changelist_hash != NULL
-      && (!status->changelist
-          || ! svn_hash_gets(eb->changelist_hash, status->changelist)))
-    return SVN_NO_ERROR; /* Filtered via changelist */
-
   /* This code does about the same thing as the inner body of
      walk_local_nodes_diff() in diff_editor.c, except that
      it is already filtered by the status walker, doesn't have to
      account for remote changes (and many tiny other details) */
 
   {
@@ -355,13 +347,12 @@ diff_status_callback(void *baton,
         /* Diff base against actual */
         if (db_kind == svn_node_file)
           {
             SVN_ERR(svn_wc__diff_base_working_diff(db, child_abspath,
                                                    child_relpath,
                                                    SVN_INVALID_REVNUM,
-                                                   eb->changelist_hash,
                                                    eb->processor,
                                                    eb->cur
                                                         ? eb->cur->baton
                                                         : NULL,
                                                    FALSE,
                                                    eb->cancel_func,
@@ -399,23 +390,21 @@ diff_status_callback(void *baton,
       {
         if (db_kind == svn_node_file)
           SVN_ERR(svn_wc__diff_local_only_file(db, child_abspath,
                                                child_relpath,
                                                eb->processor,
                                                eb->cur ? eb->cur->baton : NULL,
-                                               eb->changelist_hash,
                                                FALSE,
                                                eb->cancel_func,
                                                eb->cancel_baton,
                                                scratch_pool));
         else if (db_kind == svn_node_dir)
           SVN_ERR(svn_wc__diff_local_only_dir(db, child_abspath,
                                               child_relpath, depth_below_here,
                                               eb->processor,
                                               eb->cur ? eb->cur->baton : NULL,
-                                              eb->changelist_hash,
                                               FALSE,
                                               eb->cancel_func,
                                               eb->cancel_baton,
                                               scratch_pool));
       }
 
@@ -472,21 +461,29 @@ svn_wc__diff7(const char **root_relpath,
     *root_relpath = apr_pstrdup(result_pool,
                                 svn_dirent_skip_ancestor(eb.anchor_abspath,
                                                          local_abspath));
   if (root_is_dir)
     *root_is_dir = (kind == svn_node_dir);
 
+  /* Apply changelist filtering to the output */
+  if (changelist_filter && changelist_filter->nelts)
+    {
+      apr_hash_t *changelist_hash;
+
+      SVN_ERR(svn_hash_from_cstring_keys(&changelist_hash, changelist_filter,
+                                         result_pool));
+      diff_processor = svn_wc__changelist_filter_tree_processor_create(
+                         diff_processor, wc_ctx, local_abspath,
+                         changelist_hash, result_pool);
+    }
+
   eb.db = wc_ctx->db;
   eb.processor = diff_processor;
   eb.ignore_ancestry = ignore_ancestry;
   eb.pool = scratch_pool;
 
-  if (changelist_filter && changelist_filter->nelts)
-    SVN_ERR(svn_hash_from_cstring_keys(&eb.changelist_hash, changelist_filter,
-                                       scratch_pool));
-
   if (ignore_ancestry)
     get_all = TRUE; /* We need unmodified descendants of copies */
   else
     get_all = FALSE;
 
   /* Walk status handles files and directories */

Reply via email to