[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: parsing of boolean config options

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Sun, 15 Jun 2008 00:03:42 -0400

Branko Čibej <brane_at_xbc.nu> writes:
> Nope, config values should be consistent across all sections.

Sounds sane to me, on further thought. Shall we go with "yes"? How
about the patch below (there's more past this patch, so keep reading):

[[[
* subversion/libsvn_subr/config_file.c
  (svn_config_ensure): Just recommend "yes", don't recommend "true" as
    an alternative.
]]]

Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c (revision 31737)
+++ subversion/libsvn_subr/config_file.c (working copy)
@@ -954,9 +954,9 @@
         "### This will override the compile-time default, which is to use" NL
         "### Subversion's internal diff3 implementation." NL
         "# diff3-cmd = diff3_program (diff3, gdiff3, etc.)" NL
- "### Set diff3-has-program-arg to 'true' or 'yes' if your 'diff3'" NL
- "### program accepts the '--diff-program' option." NL
- "# diff3-has-program-arg = [true | false]" NL
+ "### Set diff3-has-program-arg to 'yes' if your 'diff3' program" NL
+ "### accepts the '--diff-program' option." NL
+ "# diff3-has-program-arg = [yes | no]" NL
         "### Set merge-tool-cmd to the command used to invoke your external" NL
         "### merging tool of choice. Subversion will pass 4 arguments to" NL
         "### the specified command: base theirs mine merged" NL

(We could also tweak ~/.subversion/README.txt, but I think it's more
important that it accurately document behavior. Most people will take
their guidance from the commented-out recommendations in the config
files themselves anyway.)

> Maybe use the right function, a.k.a. svn_config_get_bool. Or rather,
> add the equivalent server_setting variant. After all there is an int
> variant, beats me why a bool variant was never added.

D'oh, didn't even know about svn_config_get_bool(), and yes, a
svn_config_get_server_setting_bool() seems like a good idea.

The patch below adds the function; doesn't actually make use of it, I'd
prefer to do that change in a separate commit. Review welcome:

[[[
Add new function for getting boolean values from server config settings.
Along the way, factor some code and improve error reporting.

* subversion/libsvn_subr/config.c
  (get_bool): New helper function.
  (svn_config_get_bool): Rewrite as wrapper around above new helper.
  (svn_config_get_server_setting_bool): New public function.

* subversion/include/svn_config.h
  (svn_config_get_server_setting_bool): Declare it.
]]]

Index: subversion/libsvn_subr/config.c
===================================================================
--- subversion/libsvn_subr/config.c (revision 31739)
+++ subversion/libsvn_subr/config.c (working copy)
@@ -608,32 +608,57 @@
 
 
 
+/* Set *BOOLP to true or false depending (case-insensitively) on INPUT.
+ If INPUT is null, set *BOOLP to DEFAULT_VALUE.
+
+ INPUT is a string indicating truth or falsehood in any of the usual
+ ways: "true"/"yes"/"on"/etc, "false"/"no"/"off"/etc.
+
+ If INPUT is neither NULL nor a recognized string, return an error
+ with code SVN_ERR_BAD_CONFIG_VALUE; use SECTION and OPTION in
+ constructing the error string. */
+static svn_error_t *
+get_bool(svn_boolean_t *boolp, const char *input, svn_boolean_t default_value,
+ const char *section, const char *option)
+{
+ if (input == NULL)
+ {
+ *boolp = default_value;
+ }
+ else if (0 == svn_cstring_casecmp(input, SVN_CONFIG_TRUE)
+ || 0 == svn_cstring_casecmp(input, "yes")
+ || 0 == svn_cstring_casecmp(input, "on")
+ || 0 == strcmp(input, "1"))
+ {
+ *boolp = TRUE;
+ }
+ else if (0 == svn_cstring_casecmp(input, SVN_CONFIG_FALSE)
+ || 0 == svn_cstring_casecmp(input, "no")
+ || 0 == svn_cstring_casecmp(input, "off")
+ || 0 == strcmp(input, "0"))
+ {
+ *boolp = FALSE;
+ }
+ else
+ {
+ return svn_error_createf(SVN_ERR_BAD_CONFIG_VALUE, NULL,
+ _("Config error: invalid boolean value '%s' "
+ "in '[%s] %s'"),
+ input, section, option);
+ }
+
+ return SVN_NO_ERROR;
+}
+
+
 svn_error_t *
 svn_config_get_bool(svn_config_t *cfg, svn_boolean_t *valuep,
                     const char *section, const char *option,
                     svn_boolean_t default_value)
 {
   const char *tmp_value;
-
   svn_config_get(cfg, &tmp_value, section, option, NULL);
- if (tmp_value == NULL)
- *valuep = default_value;
- else if (0 == svn_cstring_casecmp(tmp_value, SVN_CONFIG_TRUE)
- || 0 == svn_cstring_casecmp(tmp_value, "yes")
- || 0 == svn_cstring_casecmp(tmp_value, "on")
- || 0 == strcmp(tmp_value, "1"))
- *valuep = TRUE;
- else if (0 == svn_cstring_casecmp(tmp_value, SVN_CONFIG_FALSE)
- || 0 == svn_cstring_casecmp(tmp_value, "no")
- || 0 == svn_cstring_casecmp(tmp_value, "off")
- || 0 == strcmp(tmp_value, "0"))
- *valuep = FALSE;
- else
- return svn_error_createf(SVN_ERR_BAD_CONFIG_VALUE, NULL,
- _("Config error: invalid boolean value '%s'"),
- tmp_value);
-
- return SVN_NO_ERROR;
+ return get_bool(valuep, tmp_value, default_value, section, option);
 }
 
 
@@ -918,6 +943,21 @@
   return SVN_NO_ERROR;
 }
 
+svn_error_t *
+svn_config_get_server_setting_bool(svn_config_t *cfg,
+ svn_boolean_t *valuep,
+ const char *server_group,
+ const char *option_name,
+ svn_boolean_t default_value)
+{
+ const char* tmp_value;
+ tmp_value = svn_config_get_server_setting(cfg, server_group,
+ option_name, NULL);
+ return get_bool(valuep, tmp_value, default_value,
+ server_group, option_name);
+}
+
+
 svn_boolean_t
 svn_config_has_section(svn_config_t *cfg, const char *section)
 {
Index: subversion/include/svn_config.h
===================================================================
--- subversion/include/svn_config.h (revision 31737)
+++ subversion/include/svn_config.h (working copy)
@@ -394,6 +394,24 @@
                                                apr_int64_t *result_value,
                                                apr_pool_t *pool);
 
+
+/** Set @a *valuep according to @a option_name for a given
+ * @a server_group in @a cfg, or set to @a default_value if no value is
+ * specified.
+ *
+ * Check first a default, then for an override in a server group. If
+ * a value is found but is not a valid boolean, return an
+ * SVN_ERR_BAD_CONFIG_VALUE error.
+ *
+ * @since New in 1.6.
+ */
+svn_error_t *svn_config_get_server_setting_bool(svn_config_t *cfg,
+ svn_boolean_t *valuep,
+ const char *server_group,
+ const char *option_name,
+ svn_boolean_t default_value);
+
+
 
 /** Try to ensure that the user's ~/.subversion/ area exists, and create
  * no-op template files for any absent config files. Use @a pool for any

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-15 06:04:04 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.