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

Re: [PATCH] Issue #2219 - Canonicalize values in svn:keywords

From: Madan U Sreenivasan <madan_at_collab.net>
Date: 2005-03-08 15:20:45 CET

On Tue, 2005-03-08 at 17:58, John Peacock wrote:
> Madan U Sreenivasan wrote:
> > + if (!canonicalized_value)
> > + canonicalized_value = svn_stringbuf_create (
> > + canon_table_for_keywords[canon_table_for_keywords[j].
> > + index].prop_value,
> > + pool);
>
> That code is still indented strangely (but it's so long, I don't know how much
> more can be done). Perhaps the variable names are simply too long?
> canon_keywords is probably sufficient as a variable name.
>
> > + else
> > + {
> > + svn_stringbuf_appendcstr (
> > + canonicalized_value,
> > + " ");
>
> still has odd indenting
>
> > + svn_stringbuf_appendcstr (
> > + canonicalized_value,
> > + canon_table_for_keywords[canon_table_for_keywords[j].
> > + index].prop_value);
> ...
>
> still has odd indenting
>
John,
   I agree with you about the variable names and the indentation. Will
try to do better. Thanks.
   About typedefs in the .h, that is technically the way to go... I just
wasnt sure about adding a structure that is specific only to subst.c
into svn_subst.h. What do you think, John?

   About putting the svn_subst_canonicalize_keywords() function in the
props.c and making it private...
   1. I see that there is no single place in the code where the list of
keywords exist ( say as an array ) currently all the keywords are
#define-d... which makes it difficult to add new keywords. So, if a new
keyword is added in future.... it becomes difficult to go to all the
files using the #defines to make changes. Thats why I felt all
"keyword-aware" code should be in one place...Pl. do correct me if I'm
missing out something here.
   2. Lets consider the function - svn_subst_build_keywords(). this
function operates specifically on the keywords, doing a helper kinda job
for props.c. along the same lines I feel that
svn_subst_canonicalize_keywords() could also exist in subst.c - more so,
because it is present in libsvn_subr.

Regards,
Madan.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Mar 8 15:14:38 2005

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.