Add "--xml" support to "svn diff --summarize" (Issue 2967)

Hi All,
    Attached is a patch that adds "--xml" support
for "svn diff
--summarize".  This patch includes the actual work
adding this
feature, unit tests and the Relax-NG schema file for the
output.  Here
is an example of the usage:

$ svn diff --summarize --xml -r27285:HEAD
<?xml version="1.0"?>
<diff>
<paths>
<path
   props="none"
   kind="file"
  
item="modified">subversion/libsvn_client/merge.
c</path>
<path
   props="none"
   kind="file"
  
item="modified">subversion/tests/cmdline/update
_tests.py</path>
<path
   props="none"
   kind="file"
  
item="modified">subversion/po/zh_CN.po</path
>
<path
   props="none"
   kind="file"
  
item="modified">contrib/client-side/svn-merge-v
endor.py</path>
</paths>
</diff>

As expected, below is the commit log for the suggested
patch.

Take care,

Jeremy

[[[
Add "--xml" support to "svn diff
--summarize".  (Issue 2967)

* subversion/tests/cmdline/diff_tests.py
  (diff_summarize_xml): Created unit test to test
"--xml" flag functionality.
  (test_list): Added diff_summarize_xml to the list of tests
to run for diff.

* subversion/tests/cmdline/svntest/actions.py
  (run_and_verify_diff_summarize_xml): Added a convenience
function
  for testing the new "--xml" flag funtionality.

* subversion/svn/diff-cmd.c
  (kind_to_char): Refactored text_mod_char to kind_to_char
for readability.
  Translates a svn_client_diff_summarize_kind_t into a char
representation.
  (kind_to_word): Translates a
svn_client_diff_summarize_kind_t to a word
  representation.
  (summarize_func_xml): New function for outputting the
"diff --summarize"
  in XML format.
  (summarize_func): Updated to use kind_to_char instead of
text_mod_char.
  (svn_cl__diff): Added logic to handle the
"--xml" flag.

* subversion/svn/schema/diff.rnc
  (added to version control): This is the Relax-NG schema
for the XML output.

* subversion/svn/main.c
  (svn_cl__cmd_table): Updated to have the diff command
support the
  passing of the "--xml" flag.
]]]

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

Hi All,
    I have taken the feedback from Karl and created a new
patch and
accompanying log message.  Both can be found attached to
Issue 2967:

http://subversion.tigris.org/issues/show_bug.cgi?id=2967


Take care,

Jeremy

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

"Jeremy Whitlock" <jcscoobyrsgmail.com> writes:
> Hi All,
>     I have taken the feedback from Karl and created a
new patch and
> accompanying log message.  Both can be found attached
to Issue 2967:
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=2967


Comments on new patch (which has the same filename as the
previous
patch, that was a bit confusing  ):

diff_tests.py still has some overly long lines.  You can
break them
like this:

  # 1) Test --xml without --summarize
  svntest.actions.run_and_verify_svn(
    None, None,
    ".*--xml' option only valid with '--summarize'
option",
    'diff', wc_dir, '--xml')

  # 2) Test --xml on invalid revision
  svntest.actions.run_and_verify_diff_summarize_xml(
    ".*No such revision 5555555",
    wc_dir, None, None, None,
    None, '-r0:5555555')

and

  svntest.actions.run_and_verify_diff_summarize_xml([],
                                                   
os.path.join(wc_dir,
                                                            
    'iota'),
                                                    paths,
items,
                                                    props,
kinds,
                                                    '-c2')

In svntest/actions.py, you've defined a new function, which
is good,
but I think its OBJECT argument is unnecessary:

   def run_and_verify_diff_summarize_xml(error_re_string =
[],
                                         object = '',
                                         expected_paths =
[],
                                         expected_items =
[],
                                         expected_props =
[],
                                         expected_kinds =
[],
                                         *args):
     """Run 'diff --summarize --xml' with the
arguments *ARGS.  OBJECT
     is the 'diff --summarize --xml' is being ran against.
     If ERROR_RE_STRING, the ocmmand must exit with error,
and the error
     message must match regular expression ERROR_RE_STRING.
   
     Else if ERROR_RE_STRING is None, the subcommand output
will be parsed
     into an XML document and will then be verified by
comparing the parsed
     output to the contents in the EXPECTED_PATHS,
EXPECTED_ITEMS,
     EXPECTED_PROPS and EXPECTED_KINDS. Returns on success,
raises
     on failure."""
   
     output, errput = run_and_verify_svn(None, None,
error_re_string, 'diff',
                                         object,
'--summarize', '--xml', *args)

     [...]

There's a missing word in the doc string anyway , but I'd
say just
get rid of the OBJECT argument entirely.  The target can
simply be in
'args', and the options will just come before the args.

The new file subversion/svn/schema/diff.rnc had a lot of
whitespace at
the end.  Was that on purpose?

The log message doesn't need to be so verbose.  For
example:

   * subversion/tests/cmdline/diff_tests.py
     (diff_summarize_xml): Created unit test to test
"--xml"
       flag functionality.
     (test_list): Added diff_summarize_xml to the list of
tests to
       run for diff.

Obviously, the new test is for the functionality you're
implementing
in this commit, so just say "New test."  And
obviously, you're adding
that test to 'test_list', because test_list is the label for
that
second part, and obviously diff_tests.py is being "run
for diff" so...

   * subversion/tests/cmdline/diff_tests.py
     (diff_summarize_xml): New test.
     (test_list): Run it.

Similarly with other parts of the log message.

I'm not saying that the exact wording matters, just that
it's good to
avoid redundancies in the log message.  If there's a lot of
text,
someone might miss something important that really can't be
derived
from context.

-Karl

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

> I'm not saying that the exact wording matters, just
that it's good to
> avoid redundancies in the log message.  If there's a
lot of text,
> someone might miss something important that really
can't be derived
> from context.

I'll get working on this after breakfast and will have
another patch,
with proper naming convention, later today.  At least the C
stuff is
good now.  ;)

Take care,

Jeremy

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

Hi All,
    There is a new version of the patch has been attached to
Issue 2967:

http://subversion.tigris.
org/nonav/issues/showattachment.cgi/787/Issue-2967-3.patch.t
xt
(Patch Link)
http://subversion.tigris.org/issues/show_bug.cgi?id=2967
 (Issue Link)

The patch includes suggested changes to the python tests and
I have
also supplied a new commit log message per Karl's review.

Take care,

Jeremy

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org