[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] issue #2069 - "svn status" in xml mode - v4

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2005-05-23 23:23:08 CEST

Peter N. Lundblad wrote:
[...]
>>* subversion/clients/cmdline/status.c
>> (generate_status_desc): new function for getting detailed string
>> representation of status
>
> You shouldn't describe the purpose of a new function in the log. That's
> documented in the code.

I think it's often good and useful to briefly mention the purpose of a new
function in the log, as long as it is documented properly in the code as well.

>>* subversion/clients/cmdline/status-cmd.c
>> (struct status_baton): added new item 'xml_mode' in struct
>> for storing xml mode is requested or not
>> (print_header_xml): prints xml header
>> (print_footer_xml): prints xml footer
>> (print_against_xml): prints status against version details in xml
>
> The three above functions are no. Please say so in the log.

I think Peter means "new".

>> (print_status): checks xml option and calls xml handling routine,
>>
>>* subversion/clients/cmdline/dtd/status.dtd
>> added a new dtd file for validating status xml
>
> "New file." is enough here.

Yes, that would be enough, though I wouldn't object to the few words of
description.

[...]
>>+ /* "<entry>" */
>>+ svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "entry",
>>+ "path", path[0] == '\0' ? "." : path, NULL);
>
> svn_path_local_style takes care of this special casing for the empty
> path. You should use that, since this comes from a local path
> originally.

I don't think we want backslashes in the XML when on Windows, for example, do we?

>> /* Called by status-cmd.c */
>> svn_error_t *
>>+svn_cl__print_status_xml (const char *path,
>>+ svn_wc_status2_t *status,
>>+ svn_boolean_t detailed,
>>+ svn_boolean_t show_last_committed,
>>+ svn_boolean_t skip_unrecognized,
>>+ svn_boolean_t repos_locks,
>>+ apr_pool_t *pool)
>>+{
[...]
>>+ return print_status_xml (svn_path_local_style (path, pool),

... So do we really want to call svn_path_local_style?

>>+ detailed, show_last_committed,
>>+ repos_locks, status, pool);
>>+}

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon May 23 23:24:16 2005

This is an archived mail posted to the Subversion Dev mailing list.