You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by John Peacock <jp...@rowman.com> on 2005/01/15 01:57:30 UTC
[PATCH] Keywords as hash - Final(?) submission
Attached is the last (I hope) patch in the multiple iterations towards mostly
closing Issue #890. This includes tests for the new $UUID$ keyword and is
presented as a single diff against trunk as of r12736. Commit log inlined and
attached.
John
====
Implement printf-like format characters for keyword expansion. Change all
other libraries to use new keyword hash functions from libsvn_subst
See issue #890 for details.
* includes/svn_subst.h:
deprecate svn_subst_keywords_t
(svn_subst_build_keywords2): Interface change. A new argument
const char *uuid.
(svn_subst_build_keywords): API compatibility wrapper for previous
(svn_subst_keywords_differ2): Interface change. A new argument
apr_pool_t *pool and use hash instead of struct.
(svn_subst_keywords_differ): API compatibility wrapper for previous
(svn_subst_copy_and_translate3): Interface change; hash instead of struct
(svn_subst_copy_and_translate2): compatibility wrapper for translate3
(svn_subst_copy_and_translate): compatibility wrapper for translate3
(svn_subst_translate_cstring2): Interface change; hash instead of struct
(svn_subst_translate_cstring): API compatibility wrapper for previous
(svn_subst_translate_stream2): Interface change; hash instead of struct
(svn_subst_translate_stream): API compatibility wrapper for previous
* includes/svn_types.h:
Define keyword format string for Revision, Date, Author, URL, UUID, ID
* libsvn_subr/svn_subst.c:
(keyword_printf): New private function; printf-style formatting of
keywords based on format strings
(keywords_to_keyhash): New private function; convert keywords struct into
keywords hash
(keyhash_to_keywords): New private function; convert keywords hash into
keywords struct
(svn_subst_build_keywords): API combatibility wrapper
(svn_subst_build_keywords2): Build keywords using keyword_printf().
(translate_keyword): Interface changes. It now looks up keyword
passed in buffer, instead of predefined constant string.
(svn_subst_translate_stream): API combatibility wrapper
(svn_subst_translate_stream2): Loop over all elements of keyword hash
instead of structure elements.
(svn_subst_keywords_differ): API combatibility wrapper
(svn_subst_keywords_differ2): Compare two hashes instead of comparing
individual structure elements.
(svn_subst_copy_and_translate): API combatibility wrapper
(svn_subst_copy_and_translate2): API combatibility wrapper
(svn_subst_copy_and_translate3): uses keywords hash instead of struct
* subversion/libsvn_wc/merge.c
(svn_wc_merge): use key hashes, new svn_wc__get_keywords() parameters,
and svn_subst_copy_and_translate2()
* subversion/libsvn_wc/translate.h
(svn_wc__get_keywords): change signature to use keyword hash
* subversion/libsvn_wc/props.c
(validate_eol_prop_against_file):
(svn_wc_prop_set): use svn_subst_translate_stream2(), keyword hashes,
and new svn_wc__get_keywords()
* subversion/libsvn_wc/adm_crawler.c
(restore_file): use keyword hashes, svn_subst_copy_and_translate3(),
and new svn_wc__get_keywords()
* subversion/libsvn_wc/log.c
(file_xfer_under_path):
(install_committed_file): use keyword hashes, new svn_wc__get_keywords(),
and svn_subst_copy_and_translate3()
* subversion/libsvn_wc/adm_ops.c
(revert_admin_things): use keyword hashes, new svn_wc__get_keywords(),
and svn_subst_copy_and_translate3()
* subversion/libsvn_wc/translate.c
(svn_wc_translated_file): use keyword hashes, new svn_wc__get_keywords(),
and svn_subst_copy_and_translate3()
(svn_wc__get_keywords): change signature to use keyword hashes and
svn_subst_build_keywords2()
* subversion/libsvn_client/export.c
(copy_one_versioned_file): use keyword hashes, new svn_wc__get_keywords(),
and svn_subst_copy_and_translate3()
(copy_versioned_files): retrieve uuid for call to copy_one_versioned_file
(struct edit_baton): add uuid
(struct file_baton): add uuid
(add_file): copy uuid from edit baton to file baton during initialization
(close_file): initialize keyword hash, populate it with
svn_subst_build_keywords2(), and call svn_subst_copy_and_translate3()
(svn_client_export3): retrieve uuid and store in edit baton
* subversion/libsvn_client/cat.c
(svn_client_cat2): use keyword hashes, svn_subst_build_keywords2(),
and svn_subst_translate_stream2()
* subversion/libsvn_client/commit.c
(send_file_contents): use keyword hashes, svn_subst_build_keywords2(),
and svn_subst_translate_stream2()
* subversion/clients/cmdline/util.c
(svn_cl__edit_externally): use new svn_subst_translate_cstring2()
* subversion/tests/libsvn_wc/translate-test.c
(substitute_and_verify): use keyword hashes and add uuid parameter
(expand_uuid and unexpand_uuid): new tests for UUID keyword
(multiple test cases): reformat parameters and add uuid
--
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
Re: [PATCH] Keywords as hash - Final(?) submission
Posted by John Peacock <jp...@rowman.com>.
Philip Martin wrote:
> Apart from some interface queries my comments are mainly whitespace
> issues. I don't have any strong feelings about issue 890 (I don't
> use keywords) but the code looks reasonable.
Thanks for the detailed criticism. I hadn't thought of apr_hash_t** being that
much more efficient, but you are right, that would be a better API. In all
honesty, some of the less efficient bits of this are due to the incremental
changes to the patch over time. If the compatibility functions have to allocate
a pool (since they don't have one to give), that was just something that I had
to live with, for example.
I'll go over your suggestions and implement them (don't know if it will be
tonight). I'll also add the changes to the svn propset help text (forgot about
that one).
Thanks again!
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
Re: [PATCH] Keywords as hash - Final(?) submission
Posted by Julian Foad <ju...@btopenworld.com>.
John Peacock wrote:
> Julian Foad wrote:
>
>> One thing you could do if people don't like it is keep the existing
>> implementation of that function (which doesn't need a pool) rather
>> than changing it to a wrapper around the new function.
>
> I don't see how that is possible; as a deprecated function, I have to
> support it to the best of my ability using the new functionality, since
> there is no way to support it with the old (the data doesn't exist that
> way any longer).
Sorry - I didn't specify which function I was talking about. I was talking
about "svn_subst_keywords_differ" which doesn't call any other functions so
could just be kept as-is.
There is also "svn_subst_translate_stream" which does call other functions, and
it wouldn't be so straightforward to keep it. As you imply below, you'd also
have to keep some other functions on which it depends.
> Or are you suggesting that I keep _all_ of the old
> functions around for compatibility purposes, and have API/code
> duplication instead of compatility wrappers?
No, it's fine to keep just the minimum necessary to achieve compatibility.
> As a deprecated function, however, I expect fewer callers to it over
> time (actually as a function which probably should not have been part of
> the public interface, I don't expect _any_ callers to it). So the minor
> /faux pax/ of creating a top-level pool with a very brief lifespan
> should be a forgivable limitation.
Agreed, unless someone tells us that a top-level pool is worse than just a
minor /faux pas/.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Keywords as hash - Final(?) submission
Posted by John Peacock <jp...@rowman.com>.
Julian Foad wrote:
> One thing you could do if people don't like it is keep the existing
> implementation of that function (which doesn't need a pool) rather than
> changing it to a wrapper around the new function.
I don't see how that is possible; as a deprecated function, I have to support it
to the best of my ability using the new functionality, since there is no way to
support it with the old (the data doesn't exist that way any longer). Or are
you suggesting that I keep _all_ of the old functions around for compatibility
purposes, and have API/code duplication instead of compatility wrappers?
As a deprecated function, however, I expect fewer callers to it over time
(actually as a function which probably should not have been part of the public
interface, I don't expect _any_ callers to it). So the minor /faux pax/ of
creating a top-level pool with a very brief lifespan should be a forgivable
limitation.
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
Re: [PATCH] Keywords as hash - Final(?) submission
Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> A new top level pool :( I think that means it bypasses the
> applications allocator, but I don't see what else you can do. I think
> we should document it at least.
One thing you could do if people don't like it is keep the existing
implementation of that function (which doesn't need a pool) rather than
changing it to a wrapper around the new function.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Keywords as hash - Final(?) submission
Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:
> Apart from some interface queries my comments are mainly whitespace
> issues. I don't have any strong feelings about issue 890 (I don't
> use keywords) but the code looks reasonable.
Something else struck me: you have introduced a new UUID keyword and I
assume the user controls it via svn:keywords, so the help text for
"svn propset" needs to be updated.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Keywords as hash - Final(?) submission
Posted by Philip Martin <ph...@codematters.co.uk>.
John Peacock <jp...@rowman.com> writes:
> --- subversion/include/svn_subst.h (/mirror/trunk) (revision 12276)
> +++ subversion/include/svn_subst.h (/local/keywords) (revision 12276)
> @@ -75,7 +75,9 @@
> const char *value);
>
>
> -/** Values used in keyword expansion. */
> +/* struct svn_subst_keywords_t is @deprecated
> +* Provided for backward compatibility with the 1.1 API.
> +*/
> typedef struct svn_subst_keywords_t
> {
> const svn_string_t *revision;
> @@ -86,8 +88,11 @@
> } svn_subst_keywords_t;
>
>
> -/** Fill in an <tt>svn_subst_keywords_t *</tt> @a kw with the appropriate
> - * contents given an @a keywords_string (the contents of the svn:keywords
> +/**
> + * @since New in 1.2.
> + *
> + * Fill in an <tt>apr_hash_t*</tt> @a kw with the appropriate contents
> + * given an @a keywords_string (the contents of the svn:keywords
> * property for the file in question), the revision @a rev, the @a url,
> * the @a date the file was committed on, and the @a author of the last
> * commit. Any of these can be @c NULL to indicate that the information is
Document the C types of the hash keys and values.
> @@ -96,6 +101,21 @@
> * All memory is allocated out of @a pool.
> */
> svn_error_t *
> +svn_subst_build_keywords2 (apr_hash_t *kw,
Any reason why you chose apr_hash_t* rather than apr_hash_t**?
> + const char *keywords_string,
> + const char *rev,
> + const char *url,
> + apr_time_t date,
> + const char *author,
> + const char *uuid,
> + apr_pool_t *pool);
> +
> +/** Similar to svn_subst_build_keywords2()
> + *
> ==================================================================
> --- subversion/libsvn_wc/merge.c (/mirror/trunk) (revision 12276)
> +++ subversion/libsvn_wc/merge.c (/local/keywords) (revision 12276)
> @@ -48,7 +48,7 @@
> const char *mt_pt, *mt_bn;
> apr_file_t *tmp_f, *result_f;
> svn_boolean_t is_binary;
> - svn_subst_keywords_t *keywords;
> + apr_hash_t *keywords = apr_hash_make(pool);
Re: [PATCH] Keywords as hash - Final(?) submission
Posted by John Peacock <jp...@rowman.com>.
Julian Foad wrote:
> I have to confess it was only seeing Philip's review that motivated me
> to have a go.
Thank you for the additional readthrough. I will lay out both Philip's and your
e-mail as I go through the code and it will be a better submission for both of
your suggestions.
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
Re: [PATCH] Keywords as hash - Final(?) submission
Posted by Julian Foad <ju...@btopenworld.com>.
I have to confess it was only seeing Philip's review that motivated me to have
a go.
John Peacock wrote:
> Implement printf-like format characters for keyword expansion. Change all
> other libraries to use new keyword hash functions from libsvn_subst
End sentences with a period? (So I'm being nit-picky :-)
> See issue #890 for details.
>
> * includes/svn_subst.h:
> deprecate svn_subst_keywords_t
"(svn_subst_keywords_t): Deprecated."
I think the idea is to parenthesise the name (symbol) of the top-level item
that was changed, whether that item was a function, a variable, a data type, or
anything else.
> (svn_subst_build_keywords2): Interface change. A new argument
> const char *uuid.
> (svn_subst_build_keywords): API compatibility wrapper for previous
> (svn_subst_keywords_differ2): Interface change. A new argument
> apr_pool_t *pool and use hash instead of struct.
> (svn_subst_keywords_differ): API compatibility wrapper for previous
> (svn_subst_copy_and_translate3): Interface change; hash instead of struct
> (svn_subst_copy_and_translate2): compatibility wrapper for translate3
> (svn_subst_copy_and_translate): compatibility wrapper for translate3
I don't think you changed this prototype (svn_subst_copy_and_translate). (It's
still documented as a wrapper for ...2, which is fine.)
> (svn_subst_translate_cstring2): Interface change; hash instead of struct
> (svn_subst_translate_cstring): API compatibility wrapper for previous
> (svn_subst_translate_stream2): Interface change; hash instead of struct
> (svn_subst_translate_stream): API compatibility wrapper for previous
>
> * includes/svn_types.h:
> Define keyword format string for Revision, Date, Author, URL, UUID, ID
"(SVN_KEYWORD_REVISION_FORMAT, SVN_KEYWORD_...): New format strings."
>
> * libsvn_subr/svn_subst.c:
> (keyword_printf): New private function; printf-style formatting of
> keywords based on format strings
> (keywords_to_keyhash): New private function; convert keywords struct into
> keywords hash
> (keyhash_to_keywords): New private function; convert keywords hash into
> keywords struct
> (svn_subst_build_keywords): API combatibility wrapper
> (svn_subst_build_keywords2): Build keywords using keyword_printf().
> (translate_keyword): Interface changes. It now looks up keyword
> passed in buffer, instead of predefined constant string.
> (svn_subst_translate_stream): API combatibility wrapper
> (svn_subst_translate_stream2): Loop over all elements of keyword hash
> instead of structure elements.
> (svn_subst_keywords_differ): API combatibility wrapper
> (svn_subst_keywords_differ2): Compare two hashes instead of comparing
> individual structure elements.
> (svn_subst_copy_and_translate): API combatibility wrapper
> (svn_subst_copy_and_translate2): API combatibility wrapper
> (svn_subst_copy_and_translate3): uses keywords hash instead of struct
>
> * subversion/libsvn_wc/merge.c
> (svn_wc_merge): use key hashes, new svn_wc__get_keywords() parameters,
> and svn_subst_copy_and_translate2()
Uses svn_subst_copy_and_translate3, not ...2?
>
> * subversion/libsvn_wc/translate.h
> (svn_wc__get_keywords): change signature to use keyword hash
>
> * subversion/libsvn_wc/props.c
> (validate_eol_prop_against_file):
> (svn_wc_prop_set): use svn_subst_translate_stream2(), keyword hashes,
> and new svn_wc__get_keywords()
>
> * subversion/libsvn_wc/adm_crawler.c
> (restore_file): use keyword hashes, svn_subst_copy_and_translate3(),
> and new svn_wc__get_keywords()
>
> * subversion/libsvn_wc/log.c
> (file_xfer_under_path):
This looks like a bit like the description is missing. You could write
"(file_xfer_under_path, install_committed_file): ..." to share a description.
> (install_committed_file): use keyword hashes, new svn_wc__get_keywords(),
> and svn_subst_copy_and_translate3()
>
> * subversion/libsvn_wc/adm_ops.c
> (revert_admin_things): use keyword hashes, new svn_wc__get_keywords(),
> and svn_subst_copy_and_translate3()
>
> * subversion/libsvn_wc/translate.c
> (svn_wc_translated_file): use keyword hashes, new svn_wc__get_keywords(),
> and svn_subst_copy_and_translate3()
> (svn_wc__get_keywords): change signature to use keyword hashes and
> svn_subst_build_keywords2()
>
> * subversion/libsvn_client/export.c
> (copy_one_versioned_file): use keyword hashes, new svn_wc__get_keywords(),
> and svn_subst_copy_and_translate3()
> (copy_versioned_files): retrieve uuid for call to copy_one_versioned_file
> (struct edit_baton): add uuid
> (struct file_baton): add uuid
> (add_file): copy uuid from edit baton to file baton during initialization
> (close_file): initialize keyword hash, populate it with
> svn_subst_build_keywords2(), and call svn_subst_copy_and_translate3()
> (svn_client_export3): retrieve uuid and store in edit baton
>
> * subversion/libsvn_client/cat.c
> (svn_client_cat2): use keyword hashes, svn_subst_build_keywords2(),
> and svn_subst_translate_stream2()
>
> * subversion/libsvn_client/commit.c
> (send_file_contents): use keyword hashes, svn_subst_build_keywords2(),
> and svn_subst_translate_stream2()
>
> * subversion/clients/cmdline/util.c
> (svn_cl__edit_externally): use new svn_subst_translate_cstring2()
>
> * subversion/tests/libsvn_wc/translate-test.c
> (substitute_and_verify): use keyword hashes and add uuid parameter
> (expand_uuid and unexpand_uuid): new tests for UUID keyword
> (multiple test cases): reformat parameters and add uuid
Inconsistent indentation in the log message.
I haven't checked the log message throughly - just noticed a few things and
skimmed through the rest.
> ------------------------------------------------------------------------
> === subversion/include/svn_subst.h
> ==================================================================
> --- subversion/include/svn_subst.h (/mirror/trunk) (revision 12276)
> +++ subversion/include/svn_subst.h (/local/keywords) (revision 12276)
> @@ -75,7 +75,9 @@
> const char *value);
>
>
> -/** Values used in keyword expansion. */
> +/* struct svn_subst_keywords_t is @deprecated
> +* Provided for backward compatibility with the 1.1 API.
> +*/
Doxygen comments need to start with "/**".
It doesn't seem like a good idea to delete the description of the item. Even
though it is deprecated, people might have valid reasons for needing to refer
to it for a while.
No need to repeat the name of the item in its comment. I think we would
usually write it like this:
/** Values used in keyword expansion.
*
* @deprecated Provided for backward compatibility with the 1.1 API.
*/
> typedef struct svn_subst_keywords_t
> {
> const svn_string_t *revision;
> @@ -86,8 +88,11 @@
> } svn_subst_keywords_t;
>
>
> -/** Fill in an <tt>svn_subst_keywords_t *</tt> @a kw with the appropriate
> - * contents given an @a keywords_string (the contents of the svn:keywords
> +/**
> + * @since New in 1.2.
> + *
> + * Fill in an <tt>apr_hash_t*</tt> @a kw with the appropriate contents
> + * given an @a keywords_string (the contents of the svn:keywords
> * property for the file in question), the revision @a rev, the @a url,
> * the @a date the file was committed on, and the @a author of the last
> * commit. Any of these can be @c NULL to indicate that the information is
> @@ -96,6 +101,21 @@
> * All memory is allocated out of @a pool.
> */
> svn_error_t *
> +svn_subst_build_keywords2 (apr_hash_t *kw,
> + const char *keywords_string,
> + const char *rev,
> + const char *url,
> + apr_time_t date,
> + const char *author,
> + const char *uuid,
> + apr_pool_t *pool);
> +
> +/** Similar to svn_subst_build_keywords2()
But you ought to state briefly what's different about it.
> + *
> + * @deprecated Provided for backward compatibility with the 1.1 API.
> + */
> +
> +svn_error_t *
> svn_subst_build_keywords (svn_subst_keywords_t *kw,
> const char *keywords_string,
> const char *rev,
> @@ -104,7 +124,6 @@
> const char *author,
> apr_pool_t *pool);
>
> -
> /** Return @c TRUE if @a a and @a b do not hold the same keywords.
> *
> * If @a compare_values is @c TRUE, "same" means that the @a a and @a b
> @@ -117,6 +136,18 @@
> * equivalent to holding no keywords.
As Philip mentioned for other functions, say something like "@a a and @a b
(hashes mapping (const char *) keyword names to (const char *) keyword
values)", or whatever the correct types and descriptions are.
> */
> svn_boolean_t
> +svn_subst_keywords_differ2 (const apr_hash_t *a,
> + const apr_hash_t *b,
> + svn_boolean_t compare_values,
> + apr_pool_t *pool);
> +
> +/** Similar to svn_subst_keywords_differ2(); provides a pool for performing
> + * the hash comparisons.
In terms of the interface and the users of this function
(svn_subst_keywords_differ), the difference regarding pools is that the user
need not provide a pool. The fact that the function creates a pool internally
is an implementation detail, and such details are not normally relevant to the
interface description. However, as Philip pointed out, the creation of a new
top-level pool is a significant thing which might be worth mentioning here.
How about:
/** Similar to svn_subst_keywords_differ2(), but keywords are passed as @c
* svn_subst_keywords_t structures, and a pool is not passed.
*
* Note: this function created and uses a temporary top-level pool.
Actually, it's not very similar to svn_subst_keywords_differ2() at all in terms
of its interface. It could just keep its old comment.
> + *
> + * @deprecated Provided for backward compatibility with the 1.1 API.
> + */
> +
> +svn_boolean_t
> svn_subst_keywords_differ (const svn_subst_keywords_t *a,
> const svn_subst_keywords_t *b,
> svn_boolean_t compare_values);
> @@ -155,6 +186,18 @@
> * convenient way to get @a eol_str and @a keywords if in libsvn_wc.
> */
> svn_error_t *
> +svn_subst_translate_stream2 (svn_stream_t *src,
> + svn_stream_t *dst,
> + const char *eol_str,
> + svn_boolean_t repair,
> + apr_hash_t *keywords,
> + svn_boolean_t expand);
> +
> +/** Similar to svn_subst_translate_stream2()
... but?
> + *
> + * @deprecated Provided for backward compatibility with the 1.1 API.
> + */
> +svn_error_t *
> svn_subst_translate_stream (svn_stream_t *src,
> svn_stream_t *dst,
> const char *eol_str,
> @@ -162,25 +205,9 @@
> const svn_subst_keywords_t *keywords,
> svn_boolean_t expand);
>
> -
> /**
> - * @deprecated Provided for backward compatibility with the 1.0 API.
> + * @since New in 1.2.
> *
> - * Similar to svn_subst_copy_and_translate2 except that @a special is
> - * always set to @c FALSE.
> - */
> -svn_error_t *
> -svn_subst_copy_and_translate (const char *src,
> - const char *dst,
> - const char *eol_str,
> - svn_boolean_t repair,
> - const svn_subst_keywords_t *keywords,
> - svn_boolean_t expand,
> - apr_pool_t *pool);
> -
> -/**
> - * @since New in 1.1.
> - *
> * Convenience routine: a variant of @c svn_subst_translate_stream
> * which operates on files. (See previous docstring for details.) In
Does that "(See previous docstring for details.)" make sense any more?
> * addition, it will create/detranslate a special file if @a special
> @@ -197,6 +224,23 @@
> * copy.
> */
> svn_error_t *
> +svn_subst_copy_and_translate3 (const char *src,
> + const char *dst,
> + const char *eol_str,
> + svn_boolean_t repair,
> + apr_hash_t *keywords,
> + svn_boolean_t expand,
> + svn_boolean_t special,
> + apr_pool_t *pool);
> +
> +/**
> + * @since New in 1.1.
> + * @deprecated Provided for backward compatibility with the 1.1 API.
> + *
> + * Similar to svn_subst_copy_and_translate3 except that struct-style
> + * keywords are converted to hash-style
> + */
> +svn_error_t *
> svn_subst_copy_and_translate2 (const char *src,
> const char *dst,
> const char *eol_str,
> @@ -206,7 +250,24 @@
> svn_boolean_t special,
> apr_pool_t *pool);
>
> -/** Convenience routine: a variant of @c svn_subst_translate_stream which
> +/**
> + * @deprecated Provided for backward compatibility with the 1.0 API.
> + *
> + * Similar to svn_subst_copy_and_translate2 except that @a special is
> + * always set to @c FALSE.
> + */
> +svn_error_t *
> +svn_subst_copy_and_translate (const char *src,
> + const char *dst,
> + const char *eol_str,
> + svn_boolean_t repair,
> + const svn_subst_keywords_t *keywords,
> + svn_boolean_t expand,
> + apr_pool_t *pool);
> +
> +/** @since New in 1.2.
> + *
> + * Convenience routine: a variant of @c svn_subst_translate_stream which
> * operates on cstrings. (See previous docstring for details.)
> *
> * Return a new string in @a *dst, allocated in @a pool, by copying the
> @@ -217,6 +278,20 @@
> * copy.
> */
> svn_error_t *
> +svn_subst_translate_cstring2 (const char *src,
> + const char **dst,
> + const char *eol_str,
> + svn_boolean_t repair,
> + apr_hash_t *keywords,
> + svn_boolean_t expand,
> + apr_pool_t *pool);
> +
> +/** @deprecated Provided for backward compatibility with the 1.1 API.
> + *
> + * Similar to svn_subst_translate_cstring2() except that keywords struct
> + * is converted to keywords hash
"it takes a keywords struct instead of a keywords hash." (The fact that it
converts it to a hash is an implementation detail; it could work equally well
without doing so.)
> + */
> +svn_error_t *
> svn_subst_translate_cstring (const char *src,
> const char **dst,
> const char *eol_str,
> === subversion/libsvn_subr/subst.c
> ==================================================================
> --- subversion/libsvn_subr/subst.c (/mirror/trunk) (revision 12276)
> +++ subversion/libsvn_subr/subst.c (/local/keywords) (revision 12276)
[...]
> @@ -528,66 +711,64 @@
> const svn_subst_keywords_t *b,
> svn_boolean_t compare_values)
> {
> + svn_boolean_t result = FALSE;
> + apr_pool_t *pool = svn_pool_create (NULL);
> +
> + /* first we have to create a hash of each of the keyword struct's */
> + apr_hash_t *ahash = keywords_to_keyhash((svn_subst_keywords_t *)a,
> + pool);
> + apr_hash_t *bhash = keywords_to_keyhash((svn_subst_keywords_t *)b,
> + pool);
> +
> + /* and then call the real function */
> + result = svn_subst_keywords_differ2 (ahash,bhash,compare_values,pool);
> +
> + /* Always clean up after ourselves */
> + svn_pool_destroy (pool);
> + return result;
> +}
> +
> +svn_boolean_t
> +svn_subst_keywords_differ2 (const apr_hash_t *a,
> + const apr_hash_t *b,
> + svn_boolean_t compare_values,
> + apr_pool_t *pool)
> +{
> + svn_boolean_t result = FALSE;
> + apr_hash_index_t *hi;
> + apr_hash_t *lame_a, *lame_b;
> +
> + lame_a = (apr_hash_t *) a;
> + lame_b = (apr_hash_t *) b;
> +
> if (((a == NULL) && (b == NULL)) /* no A or B */
> - /* no A, and B has no contents */
> - || ((a == NULL)
> - && (b->revision == NULL)
> - && (b->date == NULL)
> - && (b->author == NULL)
> - && (b->url == NULL))
> - /* no B, and A has no contents */
> - || ((b == NULL) && (a->revision == NULL)
> - && (a->date == NULL)
> - && (a->author == NULL)
> - && (a->url == NULL))
> - /* neither A nor B has any contents */
> - || ((a != NULL) && (b != NULL)
> - && (b->revision == NULL)
> - && (b->date == NULL)
> - && (b->author == NULL)
> - && (b->url == NULL)
> - && (a->revision == NULL)
> - && (a->date == NULL)
> - && (a->author == NULL)
> - && (a->url == NULL)))
Hooray for rewriting this long-winded function...
> + /* Unequal number of contents */
> + || (apr_hash_count(lame_a) != apr_hash_count(lame_b)))
> {
> - return FALSE;
> + return TRUE;
> }
... but you've changed the result of ((a == NULL) && (b == NULL)) here, now
saying that they differ.
> - else if ((a == NULL) || (b == NULL))
> - return TRUE;
> -
> - /* Else both A and B have some keywords. */
> -
> - if ((! a->revision) != (! b->revision))
> - return TRUE;
> - else if ((compare_values && (a->revision != NULL))
> - && (strcmp (a->revision->data, b->revision->data) != 0))
> - return TRUE;
> -
> - if ((! a->date) != (! b->date))
> - return TRUE;
> - else if ((compare_values && (a->date != NULL))
> - && (strcmp (a->date->data, b->date->data) != 0))
> - return TRUE;
> -
> - if ((! a->author) != (! b->author))
> - return TRUE;
> - else if ((compare_values && (a->author != NULL))
> - && (strcmp (a->author->data, b->author->data) != 0))
> - return TRUE;
> -
> - if ((! a->url) != (! b->url))
> - return TRUE;
> - else if ((compare_values && (a->url != NULL))
> - && (strcmp (a->url->data, b->url->data) != 0))
> - return TRUE;
> -
> - /* Else we never found a difference, so they must be the same. */
> -
> - return FALSE;
> +
> + /* If compare_values is FALSE, we can say A and B are the same now. */
> + if (!compare_values)
> + return FALSE;
> +
> + /* compare_values is TRUE. Compare value by value */
> + for (hi = apr_hash_first(pool, lame_a);
> + hi && !result;
> + hi = apr_hash_next(hi))
> + {
> + const char *key;
> + apr_hash_this (hi, (const void**) &key, NULL, NULL);
> + if (!svn_string_compare (apr_hash_get (lame_a, key,
> + APR_HASH_KEY_STRING),
> + apr_hash_get (lame_b, key,
> + APR_HASH_KEY_STRING)))
> + result = TRUE;
> + }
> +
> + return result;
> }
>
> -
> svn_error_t *
> svn_subst_translate_stream (svn_stream_t *s, /* src stream */
> svn_stream_t *d, /* dst stream */
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org