You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Madan U Sreenivasan <ma...@collab.net> on 2005/03/08 04:56:04 UTC
[PATCH]Issue #2219 - svn:keywords canonicalization - code, test fix
On Wed, 2005-03-02 at 21:13, kfogel@collab.net wrote:
> John Peacock <jp...@rowman.com> writes:
> > Then the book needs to change:
> >
> > http://svnbook.red-bean.com/en/1.0/ch07s02.html#svn-ch-7-sect-2.3.4
> >[...]
> > > I think we should canonicalize towards:
> > > HeadURL
> > > Author
> > > Revision
> > > Date
> > > Id
> > [...]
> -Karl
Hey Guys,
Find attached the code fix, test case and a modified existing test
case ( and of course the log )
Yet to make the doc changes....
Regards,
Madan.
Re: [PATCH]Issue #2219 - svn:keywords canonicalization - code,
test fix
Posted by Madan U Sreenivasan <ma...@collab.net>.
On Tue, 2005-03-08 at 10:26, Madan U Sreenivasan wrote:
> Find attached the code fix, test case and a modified existing test
> case ( and of course the log )
> Yet to make the doc changes....
Hey, guys, pl. ignore the above patch... am sending a new one with minor
changes... and a new test case....
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]Issue #2219 - svn:keywords canonicalization - code, test fix
Posted by kf...@collab.net.
Madan U Sreenivasan <ma...@collab.net> writes:
> You have a point there. But I'm finding it difficult to put my comments/
> acknowledgements if I put the log message in the mail. :-( . Do you have
> any suggestions? pl. let me know.
The tradition is to use three square braces "[[[ ... ]]]" to set off
the log message. Thus, patch mails traditionally look like this:
body of email message, before log message and before diff
[[[
log message here
as many lines as you need
]]]
then the diff here
Hope this helps,
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]Issue #2219 - svn:keywords canonicalization - code,
test fix
Posted by Madan U Sreenivasan <ma...@collab.net>.
On Tue, 2005-03-08 at 17:48, John Peacock wrote:
> Overall, very nice. I like how you made sure that you cannot set the same
> keyword twice (but you should really add a test for that just to show that it
> works).
There is a test for this... anyways, we shall continue on the other
thread. thanks for the input.
> It's also nicer if you include the commit message in your e-mail (i.e.
> before any attachments), just so people can see up front what they are going to
> review. My personal preference is for the order of files in the commit message
> to match up with the order of the files in the diff file (but that's just me).
You have a point there. But I'm finding it difficult to put my comments/
acknowledgements if I put the log message in the mail. :-( . Do you have
any suggestions? pl. let me know.
Thanks,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]Issue #2219 - svn:keywords canonicalization - code, test
fix
Posted by John Peacock <jp...@rowman.com>.
Madan U Sreenivasan wrote:
> Hey Guys,
> Find attached the code fix, test case and a modified existing test
> case ( and of course the log )
Overall, very nice. I like how you made sure that you cannot set the same
keyword twice (but you should really add a test for that just to show that it
works). It's also nicer if you include the commit message in your e-mail (i.e.
before any attachments), just so people can see up front what they are going to
review. My personal preference is for the order of files in the commit message
to match up with the order of the files in the diff file (but that's just me).
A couple of formatting things inline (and one philosophical argument):
> Yet to make the doc changes....
>
> Regards,
> Madan.
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/libsvn_wc/props.c
> ===================================================================
> --- subversion/libsvn_wc/props.c (revision 13300)
> +++ subversion/libsvn_wc/props.c (working copy)
> @@ -1061,7 +1061,7 @@
> }
> else if (strcmp (name, SVN_PROP_KEYWORDS) == 0)
> {
> - new_value = svn_stringbuf_create_from_string (value, pool);
> + new_value = svn_subst_canonicalize_keywords (value, pool);
> svn_stringbuf_strip_whitespace (new_value);
> }
> }
> Index: subversion/libsvn_subr/subst.c
> ===================================================================
> --- subversion/libsvn_subr/subst.c (revision 13300)
> +++ subversion/libsvn_subr/subst.c (working copy)
> @@ -47,6 +47,28 @@
> */
> #define SVN_SUBST__SPECIAL_LINK_STR "link"
>
> +/* Structure to be used for canonicalization of svn:keywords */
> +typedef struct canon_table_s
> +{
> + char prop_value[SVN_KEYWORD_MAX_LEN]; /* the keyword */
> + int canon_index; /* the corresponding canonical form */
> + int canon_flag;
> +} canon_table_t;
> +canon_table_t canon_table[] =
> + {
> + { SVN_KEYWORD_REVISION_LONG, 2, 0x0001 },
> + { SVN_KEYWORD_REVISION_SHORT, 2, 0x0001 },
> + { SVN_KEYWORD_REVISION_MEDIUM, 2, 0x0001 },
> + { SVN_KEYWORD_DATE_LONG, 4, 0x0002 },
> + { SVN_KEYWORD_DATE_SHORT, 4, 0x0002 },
> + { SVN_KEYWORD_AUTHOR_LONG, 6, 0x0004 },
> + { SVN_KEYWORD_AUTHOR_SHORT, 6, 0x0004 },
> + { SVN_KEYWORD_URL_SHORT, 7, 0x0008 },
> + { SVN_KEYWORD_URL_LONG, 7, 0x0008 },
> + { SVN_KEYWORD_ID, 9, 0x0010 }
> + };
> +int sizeof_canon_table = sizeof(canon_table)/sizeof(canon_table_t) ;
> +
> void
> svn_subst_eol_style_from_value (svn_subst_eol_style_t *style,
> const char **eol,
> @@ -118,6 +140,55 @@
> return SVN_NO_ERROR;
> }
>
> +/* Canonicalize the @a value var, assuming that it contains
> + a sequence of svn:keywords. */
Since you are creating this from scratch, I would strongly urge you to dump all
of this into props.c as a fully private function and support structures. There
is no real reason IMNSHO why all of the "keywords-aware" code has to live
exclusively in subst.c. This is strictly data validation for some properties
and as such, probably belongs in props.c. Don't take this as gospel, however,
but rather as my position (which I am prepared to argue). ;-)
If you are going to leave this in subst.c (and make it a public function), you
should really move the structure definition and add a function prototype to
subversion/include/svn_subst.h.
> +svn_stringbuf_t *
> +svn_subst_canonicalize_keywords (const svn_string_t *value,
> + apr_pool_t *pool)
> +{
> + apr_array_header_t *keyword_tokens;
> + svn_stringbuf_t *canonicalized_value = NULL ;
> + int flags = 0 ;
> + int i, j;
> +
> + /* tokenize the input */
> + keyword_tokens = svn_cstring_split (value->data, " \t\v\n\b\r\f",
> + TRUE /* chop */, pool);
> +
> + /* for all the tokens */
> + for (i = 0; i < keyword_tokens->nelts; ++i)
> + {
> + const char *keyword = APR_ARRAY_IDX (keyword_tokens, i, const char *);
> +
> + for (j = 0; j < sizeof_canon_table; j++)
> + {
> + /* see if a equivalent standard form exists */
> + if ((! strcasecmp (canon_table[j].prop_value, keyword))
> + && (! (flags & canon_table[j].canon_flag )))
> + {
> + /* If so, canonicalize and prepare output */
> + if (!canonicalized_value)
> + canonicalized_value = svn_stringbuf_create (
> + canon_table[canon_table[j].canon_index].prop_value,
> + pool);
That line is too long (by half) and the indents are wrong. Load
tools/dev/svn-dev.vim or svn-dev.el if you are running vim or emacs respectively
and reformat.
> + else
> + {
> + svn_stringbuf_appendcstr (
> + canonicalized_value,
> + " ");
wrong indent
> + svn_stringbuf_appendcstr (
> + canonicalized_value,
> + canon_table[canon_table[j].canon_index].prop_value);
wrong indent
> + }
> + flags |= canon_table[j].canon_flag ;
> + break; /* goto the next token from the input */
> + }
> + }
> + }
> +
> + return canonicalized_value ;
> +}
> +
> svn_error_t *
> svn_subst_build_keywords (svn_subst_keywords_t *kw,
> const char *keywords_val,
HTH
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