Max Bowsher wrote:
> OK, here are the results of my recent work on keywords as hash.
>
> Firstly, I've split the whole thing into 4 individually committable
> sections:
Excellent.
> 1. include & libsvn_subr
> 2. libsvn_wc
> 3. libsvn_client
> 4. cmdline client & tests
Please could you say what functional change is made in each part, not just what
files are changed? (I can understand if you haven't yet gone through
everything to determine what the changes are, but I assume at least you know
what part 1 is, and that it can safely be applied alone.)
> At the moment, I am *only* working on section 1.
[...]
> keywords_to_keyhash: Rename to kwstruct_to_kwhash. Document why we need
> to check for NULL. Convert NULL to NULL, not to an empty hash, because
> NULL is a special signifier for svn_subst_translate_stream and friends.
> [[[
> Revise keywords API - represent keywords as a hash for better
> extensibility.
> Implement printf-like format characters for keyword expansion. Change all
> other libraries to use new keyword hash functions from libsvn_subst.
> Includes a new keyword, UUID, and tests for same.
> See issue #890 for details.
> Based on a patch by John Peacock <jpeacock rowman com>.
> This is phase 1 of 4: libsvn_subr.
Are all of those changes in this patch? If not, please present a patch whose
log message just describes what it does. Then the patch will be able to stand
alone and it will not be necessary to say "this is 1 of 4" which might turn out
not to be true in the end.
> * subversion/libsvn_subr/svn_subst.c:
> (keyword_printf): New private function;
> printf-style formatting of keywords based on format strings.
> (kwstruct_to_kwhash): New private function;
> convert keywords struct into keywords hash.
> (svn_subst_build_keywords): Convert to API compatibility wrapper.
> (svn_subst_build_keywords2): Build keywords using keyword_printf().
> (translate_keyword): Interface changes. Also, look up the keyword in the
> passed in buffer, instead of trying to translate all possibilities.
> (svn_subst_keywords_differ): Retain unchanged for API compatibility.
Don't mention unchanged functions.
> (svn_subst_keywords_differ2): New function;
> compare two hashes instead of comparing individual structure elements.
> (svn_subst_translate_stream): Convert to API compatibility wrapper.
> (svn_subst_translate_stream2): Change interface only.
> (svn_subst_translate_cstring): Convert to API compatibility wrapper.
> (svn_subst_translate_cstring2): Update function call to new API version.
> (svn_subst_copy_and_translate2): Convert to API combatibility wrapper.
> (svn_subst_copy_and_translate3): Update function call to new API version.
> (svn_subst_translate_string): Update function call to new API version.
> (svn_subst_detranslate_string): Update function call to new API version.
> ]]]
I'm not sure that representing "no entries" as NULL is the right choice. It
makes tests slighly simpler, but means that every piece of code using the hash
has to test for NULL first, and every piece of code wanting to add an entry
first has to say "if (NULL) { allocate memory for the hash somewhere }" which
surely is ugly.
I'm afraid I'll have to leave this with you for a few days as I won't be able
to work on it over Easter. Thanks.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 23 14:27:35 2005