Malcolm Rowe wrote:
> On Fri, Mar 17, 2006 at 08:32:52PM +0100, Peter N. Lundblad wrote:
Re. svn_xml_is_xml_name():
>>Hmmm, would it be better to document the ASCII part as a @todo or @bug?
>>That would allow us to extend this function to cover the whole Name
>>production when someone gets around to it.
>
> I don't know whether the ASCII part is intended or not - and in any case,
> it would seem a little unreasonable to change the function so drastically.
> Maybe.
>
> The checkin that I did was just a clarification to the docstring zhakov
> introduced in r18202. Ah, I see that Julian originally introduced this
> function in r7528 - maybe he has an opinion as to what the contract
> should actually be, now that the function is public?
1. Re. semantics: For this to be a public API, your new doc string is a good
description of the implementation, and these are acceptable semantics for a
public API, but its name ought to reflect the ASCII aspect of its semantics.
At some later stage we're bound to want an "is_xml_name" with full UTF-8
semantics, and I don't think it's OK to later change the meaning that much.
Therefore let's name it something like "svn_xml_is_ascii_xml_name".
2. Re. r18202: that commit purports to avoid sharing variables
(svn_ctype_table) between libraries:
> ------------------------------------------------------------------------
> r18202 | zhakov | 2006-01-24 14:12:31 +0000 (Tue, 24 Jan 2006) | 13 lines
> Changed paths:
> M /trunk/subversion/include/svn_xml.h
> M /trunk/subversion/libsvn_client/prop_commands.c
> M /trunk/subversion/libsvn_subr/xml.c
>
> Factor out svn_xml_is_xml_name_valid() to eliminate usage of
> svn_ctype_table variable outside of libsvn_subr library. This needed because
> exporting variables from Windows DLLs requires __declspec(dllexport).
> See disscussion here: http://svn.haxx.se/dev/archive-2005-12/0178.shtml
>
> * subversion/include/svn_xml.h
> * subversion/libsvn_subr/xml.c
> (svn_xml_is_xml_name_valid): New function, factored out from
> is_valid_prop_name()
>
> * subversion/libsvn_client/prop_commands.c:
> (is_valid_prop_name): Use new function svn_xml_is_xml_name_valid()
>
> ------------------------------------------------------------------------
> Index: subversion/include/svn_xml.h
> ===================================================================
[...]
> +/** Check whether the UTF8 NAME is a the ASCII subset of an XML "Name".
> + * XML "Name" is defined at http://www.w3.org/TR/REC-xml#sec-common-syn
> + *
> + * @since New in 1.4.
> + */
> +svn_boolean_t svn_xml_is_xml_name_valid (const char *name);
That commit eliminated the single use of an svn_cytpe macro outside libsvn_subr
at the time, but did nothing to prevent such use. The email discussion said
that public access to svn_ctype_table[] was to be deprecated, which would
require the svn_ctype_* macros to be converted to functions, but that has not
been done. Now there are other uses outside libsvn_subr, so presumably the
Win32 DLLs again don't build/work properly - is that the case?
I think that commit wasn't a proper way to solve the problem. If we solve the
variable-sharing problem properly, then that commit r18202 can be reverted, and
the problem of how to name and describe the XML-name function goes away. That
would be good because we've no reason to want such a function in our public
API, as far as I know.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Mar 18 00:06:03 2006