You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2007/08/15 17:07:13 UTC
Re: [PATCH] Re: SVN & Diff & White Spaces
On Mon, Jul 30, 2007 at 06:24:52PM +0200, Stefan Sperling wrote:
> As already mentioned in a private mail to Erik I'm currently on holiday.
> So I won't be able to look into the -x parameter until the end
> of next week.
And here it is, finally.
What exactly is the difference between --ignore-space-change
and --ignore-all-space? At the moment the test I've added treats
them exactly the same. Is there a subtle difference the test should
take into account?
[[[
* subversion/svnlook/main.c:
Teach 'svnlook diff' about the --extensions parameter
as already present in the 'svn diff' subcommand.
* subversion/tests/cmdline/svnlook_tests.py:
Add two new tests, namely diff_ignore_whitespace and
diff_ignore_eolstyle, to test the new --extensions
parameter of 'svnlook diff'.
]]]
Index: subversion/svnlook/main.c
===================================================================
--- subversion/svnlook/main.c (revision 26100)
+++ subversion/svnlook/main.c (working copy)
@@ -133,6 +133,37 @@
{"version", svnlook__version, 0,
N_("show program version information")},
+#ifndef AS400
+ {"extensions", 'x', 1,
+ N_("Default: '-u'. When Subversion is invoking an\n"
+ " "
+ " external diff program, ARG is simply passed along\n"
+ " "
+ " to the program. But when Subversion is using its\n"
+ " "
+ " default internal diff implementation, or when\n"
+ " "
+ " Subversion is displaying blame annotations, ARG\n"
+ " "
+ " could be any of the following:\n"
+ " "
+ " -u (--unified):\n"
+ " "
+ " Output 3 lines of unified context.\n"
+ " "
+ " -b (--ignore-space-change):\n"
+ " "
+ " Ignore changes in the amount of white space.\n"
+ " "
+ " -w (--ignore-all-space):\n"
+ " "
+ " Ignore all white space.\n"
+ " "
+ " --ignore-eol-style:\n"
+ " "
+ " Ignore changes in EOL style")},
+#endif
+
{0, 0, 0, 0}
};
@@ -166,7 +197,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, 'x'} },
{"dirs-changed", subcommand_dirschanged, {0},
N_("usage: svnlook dirs-changed REPOS_PATH\n\n"
@@ -253,6 +284,7 @@
svn_boolean_t full_paths; /* --full-paths */
svn_boolean_t copy_info; /* --copy-info */
svn_boolean_t non_recursive; /* --non-recursive */
+ const char *extensions; /* diff extension args */ /* UTF-8! */
};
@@ -271,6 +303,7 @@
svn_revnum_t rev_id;
svn_fs_txn_t *txn;
const char *txn_name /* UTF-8! */;
+ const apr_array_header_t *diff_options;
} svnlook_ctxt_t;
@@ -798,19 +831,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 +860,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 +922,48 @@
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 *opts = svn_diff_file_options_create(pool);
+
+ if (c->diff_options)
+ SVN_ERR(svn_diff_file_options_parse(opts, c->diff_options, pool));
+
+ SVN_ERR(svn_diff_file_diff_2(&diff, orig_path, new_path, opts, 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 +977,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 +1641,13 @@
baton->is_revision = opt_state->txn ? FALSE : TRUE;
baton->rev_id = opt_state->rev;
baton->txn_name = apr_pstrdup(pool, opt_state->txn);
+
+ {
+ /* Fall back to "" to get diff_options initialized either way. */
+ const char *optstr = opt_state->extensions ? opt_state->extensions : "";
+ baton->diff_options = svn_cstring_split(optstr, " \t\n\r", TRUE, pool);
+ }
+
if (baton->txn_name)
SVN_ERR(svn_fs_open_txn(&(baton->txn), baton->fs,
baton->txn_name, pool));
@@ -2060,6 +2111,10 @@
opt_state.copy_info = TRUE;
break;
+ case 'x':
+ opt_state.extensions = opt_arg;
+ 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 26100)
+++ subversion/tests/cmdline/svnlook_tests.py (working copy)
@@ -415,7 +415,110 @@
if len(history[2:]) != 1:
raise svntest.Failure("Output not limited to expected number of items")
+#----------------------------------------------------------------------
+def diff_ignore_whitespace(sbox):
+ "test 'svnlook diff -x -b' and 'svnlook diff -x -w'"
+ 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 -x --ignore-space-change' on mu.
+ # It should not print anything.
+ output = run_svnlook('diff', '-r2', '-x', '--ignore-space-change',
+ repo_dir, '/A/mu')
+ if output != []:
+ raise svntest.Failure
+
+ # Check the output of 'svnlook diff -x --ignore-all-space' on mu.
+ # It should not print anything.
+ output = run_svnlook('diff', '-r2', '-x', '--ignore-all-space',
+ repo_dir, '/A/mu')
+ if output != []:
+ raise svntest.Failure
+
+#----------------------------------------------------------------------
+def diff_ignore_eolstyle(sbox):
+ "test 'svnlook diff -x --ignore-eol-style'"
+
+ sbox.build()
+ repo_dir = sbox.repo_dir
+ wc_dir = sbox.wc_dir
+
+ if os.name == 'nt':
+ crlf = '\n'
+ else:
+ crlf = '\r\n'
+
+ mu_path = os.path.join(wc_dir, 'A', 'mu')
+
+ rev = 1
+ # do the --ignore-eol-style test for each eol-style
+ for eol, eolchar in zip(['CRLF', 'CR', 'native', 'LF'],
+ [crlf, '\015', '\n', '\012']):
+ # rewrite file mu and set the eol-style property.
+ svntest.main.file_write(mu_path, "This is the file 'mu'." + eolchar, 'wb')
+ svntest.main.run_svn(None, 'propset', 'svn:eol-style', eol, mu_path)
+
+ # 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
+ # revision 1, but mu should be at revision rev + 1.
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.tweak('A/mu', wc_rev=rev + 1)
+
+ svntest.actions.run_and_verify_commit(wc_dir,
+ expected_output,
+ expected_status,
+ None,
+ None, None,
+ None, None,
+ wc_dir)
+
+ # Grab the diff
+ expected_output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'diff',
+ '-r', 'PREV',
+ '-x',
+ '--ignore-eol-style',
+ mu_path)
+
+ output = run_svnlook('diff', '-r%i' % (rev + 1), '-x', '--ignore-eol-style',
+ repo_dir, '/A/mu')
+ rev += 1
+
+ # replace wcdir/A/mu with A/mu in expected_output
+ for i in xrange(len(expected_output)):
+ expected_output[i] = expected_output[i].replace(mu_path, 'A/mu')
+
+ svntest.actions.compare_and_display_lines('', '', expected_output, output)
+
+
########################################################################
# Run the tests
@@ -429,6 +532,8 @@
changed_copy_info,
tree_non_recursive,
limit_history,
+ diff_ignore_whitespace,
+ diff_ignore_eolstyle,
]
if __name__ == '__main__':
--
Stefan Sperling <st...@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
Re: [PATCH] Re: SVN & Diff & White Spaces
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Aug 16, 2007 at 12:33:02PM -0400, David Glasser wrote:
> On 8/15/07, Stefan Sperling <st...@elego.de> wrote:
> > What exactly is the difference between --ignore-space-change
> > and --ignore-all-space? At the moment the test I've added treats
> > them exactly the same. Is there a subtle difference the test should
> > take into account?
>
> I believe the difference is that a change from "a b" to "a
> b" is ignored by either, but a change from "ab" to "a b" is only
> ignored by --ignore-all-space.
Given that the tests for these flags in diff_tests.py are
sufficient, is it OK that my tests in svnlook_tests.py only check
for the mere presence of the --ignore-all-space and --ignore-space-change
options in 'svnlook diff', and not their functionality?
--
Stefan Sperling <st...@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
Re: [PATCH] Re: SVN & Diff & White Spaces
Posted by David Glasser <gl...@davidglasser.net>.
On 8/15/07, Stefan Sperling <st...@elego.de> wrote:
> What exactly is the difference between --ignore-space-change
> and --ignore-all-space? At the moment the test I've added treats
> them exactly the same. Is there a subtle difference the test should
> take into account?
I believe the difference is that a change from "a b" to "a
b" is ignored by either, but a change from "ab" to "a b" is only
ignored by --ignore-all-space.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org