On Mar 16, 2005, at 11:18 PM, C. Michael Pilato wrote:
> fitz@tigris.org writes:
>
>> Modified: branches/locking/subversion/include/svn_string.h
>> ======================================================================
>> ========
>> --- branches/locking/subversion/include/svn_string.h (original)
>> +++ branches/locking/subversion/include/svn_string.h Wed Mar 16
>> 22:56:09 2005
>> @@ -320,6 +320,13 @@
>> svn_boolean_t svn_cstring_match_glob_list (const char *str,
>> apr_array_header_t *list);
>>
>> +/** Return the number of lines in @a msg, allowing any kind of
>> newline
>> + * termination (CR, CRLF, or LFCR), even inconsistent. The minimum
>> + * number of lines in @a msg is 1 -- even the empty string is
>> + * considered to have one line, since we always print log messages,
>> + * lock comments, etc. with a trailing newline.
>> + */
>> +int svn_cstring_count_newlines (const char *msg);
>
> Er. I know you just moved this code from elsewhere, but if the
> function's name is ...count_newlines(), then the function should count
> newlines, period. If that means that callers have to first tack on
> that trailing newline that they always print and *then* ask the
> function to count newlines, so be it. But a function like this simply
> should not have a usage message that long.
Agreed. Fixed in r13453.
>> Modified: branches/locking/subversion/svnadmin/main.c
>> ======================================================================
>> ========
>> --- branches/locking/subversion/svnadmin/main.c (original)
>> +++ branches/locking/subversion/svnadmin/main.c Wed Mar 16 22:56:09
>> 2005
>> @@ -996,14 +996,13 @@
>> /* Fetch all locks on or below the root directory. */
>> SVN_ERR (svn_repos_fs_get_locks (&locks, repos, "/", NULL, NULL,
>> pool));
>>
>> - SVN_ERR (svn_cmdline_printf (pool, "\n"));
>> -
>> for (hi = apr_hash_first (pool, locks); hi; hi = apr_hash_next
>> (hi))
>> {
>> const void *key;
>> void *val;
>> const char *path, *cr_date, *exp_date;
>> svn_lock_t *lock;
>> + int comment_lines;
>>
>> apr_hash_this (hi, &key, NULL, &val);
>> path = key;
>> @@ -1028,8 +1027,12 @@
>> _(" Created: %s\n"),
>> cr_date));
>> SVN_ERR (svn_cmdline_printf (pool,
>> _(" Expires: %s\n"),
>> exp_date));
>> +
>> + comment_lines = svn_cstring_count_newlines (lock->comment);
>> SVN_ERR (svn_cmdline_printf (pool,
>> - _(" Comment: %s\n\n"),
>> + _(" Comment: (%i
>> %s)\n%s\n"),
>> + comment_lines,
>> + (comment_lines > 1) ? "lines" :
>> "line",
>> lock->comment ? lock->comment :
>> "none"));
>
> Two other complaints I raised to Ben earlier today (and which I will
> be fixing as soon as I complete this mail):
>
> The locked path should be output as "Path: /the/path", just like
> the other values.
>
> All the lines of output from lslocks should be left-justified. Not
> only is this consistent with the likes of 'svn info', but the "line
> up the colons" output right now jeopardizes our ability to add
> stuff in the future that would need more room on the left-hand side
> of that colon:
>
> $ svnadmin lslocks repos
>
> /foo
> UUID Token:
> opaquelocktoken:2d5d7fd2-77f2-0310-bd81-80b5e1c270c3
> Owner: cmpilato
> Created: 2005-03-16 19:02:56 -0600 (Wed, 16 Mar 2005)
> Expires: never
> Comment: boo
> SomeReallyLongThing: value
>
> Knowwhutimean, Vern?
>
> Anyway, like I said, I'll fix the lslocks output. You can tweak the
> implementation and API docs for your new public function.
+1. Go right ahead.
-Fitz
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 17 07:06:38 2005