Re: [PATCH] Re: SVN & Diff & White Spaces

On Fri, Jul 27, 2007 at 02:58:40PM +0300, tom wrote:
> Sorry, have no chance atm.
> We've 1.3.2 installed, going move to 1.5 after release.

Well, I don't expect you to test this on a production
system of course :)

But if you have no time to test, that's OK.

@list: Should I create an issue for this so it does not get
       lost in the mailing list archives?

I've added a test for this feature as well now, updated diff below.

[[[

Implement svnlook diff --ignore-whitespace.

* subversion/svnlook/main.c:

  (print_diff_tree): We should only print a header if we have
   a corresponding diff to print. So create the diff header in
   a buffer instead of printing it straight to stdout. It might
   turn out the diff is empty if --ignore-whitespace is specified!
   Also, while here, fix a spurious newline being printed even if
   there is nothing else interesting to print.

* subversion/tests/cmdline/svnlook_tests.py:
   (diff_ignore_whitespace): Add test for diff --ignore-whitespace.

]]]

Index: subversion/svnlook/main.c
===================================================================
--- subversion/svnlook/main.c (revision 25859)
+++ subversion/svnlook/main.c (working copy)
@@ -80,7 +80,8 @@
     svnlook__diff_copy_from,
     svnlook__revprop_opt,
     svnlook__full_paths,
- svnlook__copy_info
+ svnlook__copy_info,
+ svnlook__diff_ignore_space
   };
 
 /*
@@ -133,6 +134,9 @@
   {"version", svnlook__version, 0,
    N_("show program version information")},
 
+ {"ignore-whitespace", svnlook__diff_ignore_space, 0,
+ N_("ignore whitespace")},
+
   {0, 0, 0, 0}
 };
 
@@ -166,7 +170,7 @@
    N_("usage: svnlook diff REPOS_PATH\n\n"
       "Print GNU-style diffs of changed files and properties.\n"),
    {'r', 't', svnlook__no_diff_deleted, svnlook__no_diff_added,
- svnlook__diff_copy_from} },
+ svnlook__diff_copy_from, svnlook__diff_ignore_space} },
 
   {"dirs-changed", subcommand_dirschanged, {0},
    N_("usage: svnlook dirs-changed REPOS_PATH\n\n"
@@ -253,6 +257,7 @@
   svn_boolean_t full_paths; /* --full-paths */
   svn_boolean_t copy_info; /* --copy-info */
   svn_boolean_t non_recursive; /* --non-recursive */
+ svn_boolean_t diff_ignore_space;/* --ignore-whitespace */
 };
 
 
@@ -271,6 +276,7 @@
   svn_revnum_t rev_id;
   svn_fs_txn_t *txn;
   const char *txn_name /* UTF-8! */;
+ svn_boolean_t diff_ignore_space;
 
 } svnlook_ctxt_t;
 
@@ -798,19 +804,23 @@
   svn_boolean_t do_diff = FALSE;
   svn_boolean_t orig_empty = FALSE;
   svn_boolean_t is_copy = FALSE;
- svn_boolean_t printed_header = FALSE;
   svn_boolean_t binary = FALSE;
   apr_pool_t *subpool;
+ svn_stringbuf_t *header;
 
   SVN_ERR(check_cancel(NULL));
 
   if (! node)
     return SVN_NO_ERROR;
 
+ header = svn_stringbuf_create("", pool);
+
   /* Print copyfrom history for the top node of a copied tree. */
   if ((SVN_IS_VALID_REVNUM(node->copyfrom_rev))
       && (node->copyfrom_path != NULL))
     {
+ svn_string_t *str;
+
       /* This is ... a copy. */
       is_copy = TRUE;
 
@@ -823,11 +833,10 @@
       else
         base_path = apr_pstrdup(pool, node->copyfrom_path);
 
- SVN_ERR(svn_cmdline_printf(pool, _("Copied: %s (from rev %ld, %s)\n"),
- path, node->copyfrom_rev, base_path));
+ str = svn_string_createf(pool, _("Copied: %s (from rev %ld, %s)\n"),
+ path, node->copyfrom_rev, base_path);
+ svn_stringbuf_appendcstr(header, str->data);
 
- printed_header = TRUE;
-
       SVN_ERR(svn_fs_revision_root(&base_root,
                                    svn_fs_root_fs(base_root),
                                    node->copyfrom_rev, pool));
@@ -886,37 +895,49 @@
                                    tmpdir, pool));
         }
 
- /* The header for the copy case has already been written, and we don't
+ /* The header for the copy case has already been created, and we don't
          want a header here for files with only property modifications. */
- if (! printed_header
+ if (header->len == 0
           && (node->action != 'R' || node->text_mod))
         {
- SVN_ERR(svn_cmdline_printf(pool, "%s: %s\n",
+ svn_string_t *str;
+
+ str = svn_string_createf(pool, "%s: %s\n",
                                      ((node->action == 'A') ? _("Added") :
                                       ((node->action == 'D') ? _("Deleted") :
                                        ((node->action == 'R') ? _("Modified")
                                         : _("Index")))),
- path));
- printed_header = TRUE;
+ path);
+ svn_stringbuf_appendcstr(header, str->data);
         }
     }
 
   if (do_diff)
     {
- SVN_ERR(svn_cmdline_printf(pool, "%s\n", equal_string));
- SVN_ERR(svn_cmdline_fflush(stdout));
+ svn_stringbuf_appendcstr(header, equal_string);
+ svn_stringbuf_appendcstr(header, "\n");
 
       if (binary)
- SVN_ERR(svn_cmdline_printf(pool, _("(Binary files differ)\n")));
+ svn_stringbuf_appendcstr(header, _("(Binary files differ)\n\n"));
       else
         {
           svn_diff_t *diff;
- SVN_ERR(svn_diff_file_diff(&diff, orig_path, new_path, pool));
+ svn_diff_file_options_t *diff_options =
+ svn_diff_file_options_create(pool);
+
+ if (c->diff_ignore_space)
+ diff_options->ignore_space = svn_diff_file_ignore_space_all;
+
+ SVN_ERR(svn_diff_file_diff_2(&diff, orig_path, new_path,
+ diff_options, pool));
           if (svn_diff_contains_diffs(diff))
             {
               svn_stream_t *ostream;
               const char *orig_label, *new_label;
 
+ /* Print diff header. */
+ SVN_ERR(svn_cmdline_printf(pool, header->data));
+
               SVN_ERR(svn_stream_for_stdout(&ostream, pool));
               
               if (orig_empty)
@@ -930,14 +951,11 @@
                        orig_label, new_label,
                        svn_cmdline_output_encoding(pool), pool));
               SVN_ERR(svn_stream_close(ostream));
+ SVN_ERR(svn_cmdline_printf(pool, "\n"));
             }
         }
-
- SVN_ERR(svn_cmdline_printf(pool, "\n"));
       SVN_ERR(svn_cmdline_fflush(stdout));
     }
- else if (printed_header)
- SVN_ERR(svn_cmdline_printf(pool, "\n"));
 
   /* Make sure we delete any temporary files. */
   if (orig_path)
@@ -1597,6 +1615,7 @@
   baton->is_revision = opt_state->txn ? FALSE : TRUE;
   baton->rev_id = opt_state->rev;
   baton->txn_name = apr_pstrdup(pool, opt_state->txn);
+ baton->diff_ignore_space = opt_state->diff_ignore_space;
   if (baton->txn_name)
     SVN_ERR(svn_fs_open_txn(&(baton->txn), baton->fs,
                             baton->txn_name, pool));
@@ -2060,6 +2079,10 @@
           opt_state.copy_info = TRUE;
           break;
 
+ case svnlook__diff_ignore_space:
+ opt_state.diff_ignore_space = TRUE;
+ break;
+
         default:
           subcommand_help(NULL, NULL, pool);
           svn_pool_destroy(pool);
Index: subversion/tests/cmdline/svnlook_tests.py
===================================================================
--- subversion/tests/cmdline/svnlook_tests.py (revision 25859)
+++ subversion/tests/cmdline/svnlook_tests.py (working copy)
@@ -415,7 +415,41 @@
   if len(history[2:]) != 1:
     raise svntest.Failure("Output not limited to expected number of items")
 
+def diff_ignore_whitespace(sbox):
+ "test 'svnlook diff --ignore-whitespace'"
 
+ sbox.build()
+ repo_dir = sbox.repo_dir
+ wc_dir = sbox.wc_dir
+
+ # Make whitespace-only changes to mu
+ mu_path = os.path.join(wc_dir, 'A', 'mu')
+ svntest.main.file_write(mu_path, "This is the file 'mu'.\n")
+
+ # Created expected output tree for 'svn ci'
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/mu' : Item(verb='Sending'),
+ })
+
+ # Create expected status tree; all local revisions should be at 1,
+ # but mu should be at revision 2.
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.tweak('A/mu', wc_rev=2)
+
+ svntest.actions.run_and_verify_commit(wc_dir,
+ expected_output,
+ expected_status,
+ None,
+ None, None,
+ None, None,
+ wc_dir)
+
+ # Check the output of 'svnlook diff --ignore-whitespace' on mu.
+ # It should not print anything.
+ output = run_svnlook('diff', '-r2', '--ignore-whitespace', repo_dir, '/A/mu')
+ if output != []:
+ raise svntest.Failure
+
 ########################################################################
 # Run the tests
 
@@ -429,6 +463,7 @@
               changed_copy_info,
               tree_non_recursive,
               limit_history,
+ diff_ignore_whitespace,
              ]
 
 if __name__ == '__main__':

-- 
Stefan Sperling <stsp@elego.de>                 Software Developer
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                 Geschaeftsfuehrer: Olaf Wagner