John Peacock wrote:
> Garrett Rooney wrote:
>
>> I'm not sure I like the idea of any and all properties all of a sudden
>> becoming things that can be expanded as keywords. It seems like it
>> will limit our ability to add new keywords in the future, since it
>> will break things for people who already use whatever name we decided
>> on as a property for some other use.
>
>
> I don't really expect anyone (besides me) to load this patch any time
> soon. Like I said in my intro, I think this is probably a 2.0.0 feature,
> so there is plenty of time to discuss whether/how it gets implemented.
I wouldn't necessarily say 2.0, but I do think it requires more design work.
>>
>> Perhaps the user should have to specify explicitly somewhere (in a
>> local config file now, server side config file eventually) what
>> properties are used as keywords? This would also allow the
>> possibility of specifying alternate names for existing keywords (i.e.
>> as the *BSD's use $FreeBSD$ or $NetBSD$ as alternative spellings for
>> $Id$).
>
>
> That was one of the original reasons why the original discussion came
> up. Unfortunately, it is dependent on server managed configuration
> files, which doesn't exist in Subversion at the present time. I agree
> that it is one of the more useful extensions that could be done.
Well, if you're willing to tackle that, I'm willing to review it ;-)
>>> + /* emit a warning if one has been created */
>>> + if (warn)
>>> + {
>>> + svn_handle_warning (stderr, warn);
>>> + }
>>
>>
>>
>> You can't just spew stuff to stderr inside library code. If we need a
>> way to handle a warning, you should add a callback function/baton
>> pair, so it can be called when warnings occur.
>
>
> Sorry, I thought that was what I _was_ doing by calling
> svn_handle_warning(). Care to point me to some code which does the sort
> of thing you are recommending?
I can't easily check at the moment, since svn.collab.net seems to be
down, but IIRC svn_handle_warning just prints to the filehandle (stderr
in this case) you pass it.
I don't know off the top of my head of any code that uses a
callback/baton pair for warnings (I imagine there is some... Actually,
I think the libsvn_fs stuff has a warning callback that gets called, you
might want to look there), but the idea is just to pass in a function
pointer and a void pointer to some user specified data as arguments to
the function in question, and the application gets to decide what to do
with the info you give it. In the case of the svn application it would
likely just call svn_handle_warning, but in other apps it might not.
>> I'm also not especially thrilled with the 'bail if it has a newline'
>> and 'cut it off at some arbitrary size' stuff... I suppose we can't
>> avoid the newline thing, but the arbitrary size really bothers me...
>
>
> The limits are there in the current codebase, check out this code from
> subversion/libsvn_subr/subst.c:translate_keyword_subst():
>
> ...
> /* "$keyword: value $" */
> if (vallen > (SVN_KEYWORD_MAX_LEN - 5))
> vallen = SVN_KEYWORD_MAX_LEN - 5;
> ...
>
> so the concept of truncating isn't something I came up with on my own. ;)
Interesting, I wasn't aware of that.
> I thought in both the newline and size cases, it was better to warn and
> continue than to throw an error. When I was testing this patch, I
> discovered that if I threw an error for either out-of-bounds conditions,
> it would cascade up 4 or 5 layers. For example, the code to expand
> keywords is called in libsvn_wc /after/ the file has been committed to
> the repository, so throwing an error leaves the WC in an inconsistent
> state.
Yeah, I agree that a warning is a good idea, we just need to find the
right way to do it. Perhaps a function pointer/baton pair for this
purpose could be added to the svn_client_ctx_t structure, so it could be
used in various places it is needed.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Apr 23 21:21:45 2004