[ bcc stefan2 ]

Julian Foad wrote on Wed, Jun 15, 2011 at 15:14:53 +0100:
> New in 1.7 in svn_types.h:
>   svn_tristate_t
>   svn_tristate_to_word()  # yields "true"/"false"/NULL
>   svn_tristate_from_word()  # takes "true"/"yes"/"on"/.../
> 
> The _to/from_word() functions don't seem canonically purposed: some
> users want a single representation of the values, for use in on-the-wire
> protocols and XML output, while other users want a more flexible,
> human-readable interpretation that accepts multiple different words
> ("true" = "on" = "yes").
> 
> The doc string of _from_word() is wrong: it says any input other than
> "true" and "false" is unknown.
> 
> The current users are:
> 
>   * The main use is in svn_log_changed_path2_t.text_modified
> and .props_modified.  For these, we use _to_word() in mod_dav_svn to
> write a protocol string (assumes the value is not svn_tristate_unknown),
> use _from_word() in ra_neon and ra_serf to interpret a protocol string,
> and use _to_word() in "svn log --xml" to output an XML attribute, using
> an undocumented feature of svn_xml_make_open_tag() to allow the
> attribute string to be NULL.  All of these need only "true"/"false", not
> yes/no/off/on.
> 
>   * implementation of svn_config_get_bool() and svn_hash_get_bool(),
> which uses _tristate_from_word() to flexibly read "on"/"off"/"yes"/etc.
> 
>   * svnserve:main.c which uses it simply to detect whether a
> command-line option is "true"/"yes"/"on", and doesn't actually care
> about the tri-state-ness.
> 

Why does that svnserve option take a string argument?  If it's just
a boolean why not do

-    {"cache-txdeltas", SVNSERVE_OPT_CACHE_TXDELTAS, 1, 
+    {"cache-txdeltas", SVNSERVE_OPT_CACHE_TXDELTAS, 0, 

?

> 
> At least, and at first, we should remove the second and third usages
> listed above.  Those usages are more akin to "svn_cmdline" or
> "svn_config" utilities, or could well be private.
> 
> Further, I consider moving/renaming these two functions to be a bit more
> private even for their main use in svn_log_changed_path2_t.
> 

Fair enough.  I don't see that any of the above uses require third
parties API consumers to parse tristates (from string to enum), so no
objection to making _from_word() private.

> - Julian
> 
> 

Thanks,

Reply via email to