John Peacock wrote:
> Julian Foad wrote:
>> The problem is that the keywords-as-hash API is inconsistent in how it
>> represents "no keywords": svn_subst_build_keywords2() generates an empty hash,
>> but all of the other functions expect NULL (but don't require it).
>>
>> I agonised over this all afternoon, wondering if the keywords-as-hash
>> API is acceptable, and researched the existing use of apr_hash_t in
>> our APIs, finding that we hardly ever provide or accept NULL.
>> However, the existing use of keywords did use NULL (for
>> svn_subst_keywords_t *), so I decided the API is OK as it is.
>
> An additional argument in favor of accepting NULL instead of an empty
> hash is that it is much easier to hard code NULL in internal calls where
> the keywords are not important/needed.
>
> Although we are moving to cut 1.3 on Friday, it still isn't too late to
> change svn_subst_build_keywords2() to return NULL instead of an empty
> hash, thus making the usage more consistent.
Mmm. I'm inclined to leave it as it is. Changing it in that way would add one
sort of consistency, but remove another sort: none of our other functions for
any kind of data can return null instead of an empty hash. And while it would
make testing for emptiness easier, it would make other operations (such as
looking for a specific keyword in it) harder.
What the keywords-as-hash API does now is to be conservative in what it gives
out, and forgiving in what it accepts. That's not the only right way to design
an API but it's one way of explaining the apparent mismatch as a good thing.
We can always deprecate it later if we think of something that is definitely
better.
Things we ought to do now are:
Check whether the implementations of the functions that accept a keywords hash
are reasonably efficient both when given NULL and when given an empty hash.
svn_subst_keywords_differ2(a,b) - yes
svn_subst_translate_stream3(keywords) - no
svn_subst_copy_and_translate3(keywords) - no
svn_subst_translate_cstring2(keywords) - no
Clean up svn_subst_translate_stream3()'s doc string: it refers to keywords in
the hash having null values, and to the syntax "keywords->revision".
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Sep 22 02:40:54 2005