On 7/27/07, Stefan Sperling <stsp@elego.de> wrote:
> 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?
No need, I had it marked for review in my Inbox. I must say, the patch
is very nice overall, but I have one nit. And it would be great if you
wanted to address it: the svn command takes its diff arguments as the
parameter of the -x option. It would be great if you could make it
happen for svnlook too, because of consistency across the toolset.
I can help you where to get the code from:
subversion/svn/diff-cmd.c around line 112 for processing the -x argument and
subversion/libsvn_client/diff.c around line 525 for molding the
processed argument string into the svn_diff_file_options_t structure.
Hope you can do the extra round trip!
Thanks again for the patch.
bye,
Erik.
>
> 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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jul 29 23:02:40 2007