[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: John Peacock <jpeacock_at_rowman.com>
Date: 2005-03-08 17:44:34 CET

Madan U Sreenivasan wrote:
> 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?

I can go either way, since this is technically a private structure, used only by
that one variable (which I think should probably be initialized within the
function, rather than at the file level).

> 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.

I'm working on a patch to change the way that keywords are managed (as a hash
instead of a struct); I may even have some time today (since I'm home with my
sick son) to finish it off. However, this isn't going to change the way that
the keywords are $define'd, since that is just the way that the code is
structured. Keywords are not quite large enough a task to have a dedicated
library (or even source file) to manage them.

Adding keywords is not trivial to begin with (I was able to add UUID, but not
repos_root) because of the basic design of the WC code. Worrying about
expandability here is false optimization. Better to document the code in
svn_subst.h to point to the private function in props.c, in my mind, than make a
  public function which we are stuck supporting until 2.0.

> 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.

I don't like /that/ function being public either, because it is far too
specialized for general consumption. I've argued before that there should be
semi-private functions - usable by multiple libraries in the project but only
within the project - so that the public API is more sensible.

John

-- 
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4720 Boston Way
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5747
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Mar 8 17:43:43 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.