[1165 PATCH] Request for review of basic expansion

* subversion/libsvn_subr/config.c
  (DEFAULT_SECTION): The name of the default configuration section.

  (find_option): If the option in the specified section isn't found,
    and look in the default section for an option of the same name (if
    we aren't already looking there).

  (make_string_from_option): Call expand_option_value(), and set
    opt->expanded. Expanded signature to take a cfg_section_t pointer
    for use by expand_option_value()/find_option().

  (FMT_START, FMT_END): Constants used in expand_option_value().

  (expand_option_value): New function which expands option values. It
    takes pointers to a value and x_value string -- rather than
    directly to a cfg_option_t -- so that it can be called on plain
    text (as on the default value parameter passed to
    svn_config_get()). It must be forward decl'd ahead of
    make_string_from_option(), as the functions have a bi-directional
    dependency upon each other.

    A number of questions about this function:
    - Should it handle printf-style fancy formatting? Non-string
      formats?
    - How should conditions which would be exceptions to Python's
      ConfigParser be handled: specifically, the failure to resolve an
      expansion, and an unterminated format string?
    - What's the Right Way to get the c-string data out of a
      svn_stringbuf_t? I'd like to "free" the structure after copying
      its data (I'm done with it at that point and don't see any
      reason for it to stick around in the pool).

  (svn_config_get): Pass a cfg_section_t ** to find_option() so the
    section is available when make_string_from_option() is called.
    Call expand_option_value() on the default_value parameter.

  (svn_config_enumerate): Pass a cfg_section_t * to
    make_string_from_option() to handle the function's API change.

* subversion/tests/libsvn_subr/config-test.c
  Test expansion behavior of the svn_config_read() and
  svn_config_get() functions.

  (config_keys, config_values): More name/value pairs from the
    configuration file.

  (test1): Added a conditionally compiled debugging trace. Fail the
    test if only one of the values is null.

* subversion/tests/libsvn_subr/config-test.cfg
  Introduced more features from Python's ConfigParser file format into
  test file.

Index: subversion/libsvn_subr/config.c
===================================================================
--- subversion/libsvn_subr/config.c (revision 5277)
+++ subversion/libsvn_subr/config.c (working copy)
@@ -340,6 +340,8 @@
 }
 
 
+static const char *DEFAULT_SECTION = "DEFAULT";
+
 /* Return a pointer to an option in CFG, or NULL if it doesn't exist.
    if SECTIONP is non-null, return a pointer to the option's section.
    OPTION may be NULL. */
@@ -361,26 +363,38 @@
   if (sec_ptr != NULL && option != NULL)
     {
       cfg_section_t *sec = sec_ptr;
+ cfg_option_t *opt;
 
       /* Canonicalize the option key */
       svn_stringbuf_set (cfg->tmp_key, option);
       make_hash_key (cfg->tmp_key->data);
 
- return apr_hash_get (sec->options, cfg->tmp_key->data,
- cfg->tmp_key->len);
+ opt = apr_hash_get (sec->options, cfg->tmp_key->data,
+ cfg->tmp_key->len);
+ /* NOTE: ConfigParser's sections are case sensitive. */
+ if (opt == NULL && apr_strnatcasecmp(section, DEFAULT_SECTION) != 0)
+ /* Options which aren't found in the requested section are
+ also sought after in the default section. */
+ opt = find_option (cfg, DEFAULT_SECTION, option, &sec);
+ return opt;
     }
 
   return NULL;
 }
 
 
+static void
+expand_option_value (svn_config_t *cfg, cfg_section_t *section,
+ const char *opt_value, const char **opt_x_valuep);
+
+
 /* Set *VALUEP according to the OPT's value. */
 static void
-make_string_from_option (const char **valuep,
- svn_config_t *cfg, cfg_option_t *opt)
+make_string_from_option (const char **valuep, svn_config_t *cfg,
+ cfg_section_t *section, cfg_option_t *opt)
 {
- /* ### TODO: Expand the option's value */
- (void)(cfg);
+ expand_option_value (cfg, section, opt->value, &opt->x_value);
+ opt->expanded = TRUE;
 
   /* For legacy reasons, the cfg is still using counted-length strings
      internally. But the public interfaces just use null-terminated
@@ -393,6 +407,78 @@
 }
 
 
+#define FMT_START "%("
+#define FMT_END ")s"
+
+
+static void
+expand_option_value (svn_config_t *cfg, cfg_section_t *section,
+ const char *opt_value, const char **opt_x_valuep)
+{
+ svn_stringbuf_t *buf = NULL;
+ const char *remaining_value = opt_value;
+ const char *name_start, *name_end;
+
+ /* ### Handle fancy/non-string format specifiers? */
+ while (remaining_value != NULL
+ && (name_start = strstr (remaining_value, FMT_START)) != NULL)
+ {
+ name_start += strlen (FMT_START);
+ name_end = strstr (name_start, FMT_END);
+
+ if (name_end != NULL)
+ {
+ cfg_option_t *x_opt;
+ int name_len = name_end - name_start;
+ char name[name_len + 1];
+
+ apr_cpystrn (name, name_start, name_len + 1);
+ x_opt = find_option (cfg, section->name, name, NULL);
+
+ if (x_opt != NULL)
+ {
+ const char *cstring;
+ make_string_from_option (&cstring, cfg, section, x_opt);
+
+ if (buf == NULL)
+ {
+ int n = name_start - strlen (FMT_START) - remaining_value;
+ buf = svn_stringbuf_ncreate(remaining_value, n,
+ cfg->x_pool);
+ cfg->x_values = TRUE;
+ }
+ svn_stringbuf_appendcstr (buf, cstring);
+ remaining_value = name_end + strlen(FMT_END);
+ name_start = strstr (remaining_value, FMT_START);
+ if (name_start == NULL)
+ {
+ /* Done looping, no more expansions to process. */
+ svn_stringbuf_appendcstr (buf, remaining_value);
+ remaining_value = NULL;
+ }
+ }
+ else
+ /* Though ConfigParser considers the failure to resolve
+ the requested expansion an exception condition, we
+ leave the expanded value set to NULL. */
+ remaining_value = NULL;
+ }
+ else
+ /* Though ConfigParser treats unterminated format specifiers
+ as an exception condition, we leave the expanded value set
+ to NULL. */
+ remaining_value = NULL;
+ }
+
+ if (buf != NULL)
+ {
+ /* ### HELP: What to do with buf? We want its c-string data,
+ ### not the struct itself. */
+ *opt_x_valuep = buf->data;
+ }
+}
+
+
 
 void
 svn_config_get (svn_config_t *cfg, const char **valuep,
@@ -401,11 +487,12 @@
 {
   if (cfg)
     {
- cfg_option_t *opt = find_option (cfg, section, option, NULL);
+ cfg_section_t *sec;
+ cfg_option_t *opt = find_option (cfg, section, option, &sec);
       if (opt != NULL)
- make_string_from_option (valuep, cfg, opt);
+ make_string_from_option (valuep, cfg, sec, opt);
       else
- *valuep = default_value; /* ### TODO: Expand default_value */
+ expand_option_value (cfg, sec, default_value, valuep);
     }
   else
     *valuep = default_value;
@@ -481,7 +568,7 @@
       opt = opt_ptr;
 
       ++count;
- make_string_from_option (&temp_value, cfg, opt);
+ make_string_from_option (&temp_value, cfg, sec, opt);
       if (!callback (opt->name, temp_value, baton))
         break;
     }
Index: subversion/tests/libsvn_subr/config-test.cfg
===================================================================
--- subversion/tests/libsvn_subr/config-test.cfg (revision 5277)
+++ subversion/tests/libsvn_subr/config-test.cfg (working copy)
@@ -1,3 +1,24 @@
+# default values across all sections
+[DEFAULT]
+foo=bar
+# Not implementing __name__ expansions
+#baz=%(__name__)s
+
 [section1]
+# Trailing whitespace
 a=Aa
-b= 100
\ No newline at end of file
+# leading whitespace / numeric
+b= 100
+# Variable expansion
+c=%(foo)s
+# Expansion for non-existent option (ConfigParser throws an
+# InterpolationError with the message "Bad value substitution")
+d=%(bogus)s
+# Expansion format escaping doesn't seem possible
+e=%%(a)s
+# Two-level variable expansion with surrounding text
+f=lyrical %(c)sd
+# Unterminated format string
+g= %(unterminated
+# Multiple expansions
+h=%(a)s %(b)s
\ No newline at end of file
Index: subversion/tests/libsvn_subr/config-test.c
===================================================================
--- subversion/tests/libsvn_subr/config-test.c (revision 5277)
+++ subversion/tests/libsvn_subr/config-test.c (working copy)
@@ -84,10 +84,12 @@
 }
 
 
-static const char *config_keys[] = { "a", "b", NULL };
-static const char *config_values[] = { "Aa", "100", NULL };
+static const char *config_keys[] = { "foo", "a", "b", "c", "d", "e", "f", "g",
+ "h", NULL };
+static const char *config_values[] = { "bar", "Aa", "100", "bar", "%(bogus)s",
+ "%Aa", "lyrical bard",
+ "%(unterminated", "Aa 100", NULL };
 
-
 static svn_error_t *
 test1 (const char **msg,
        svn_boolean_t msg_only,
@@ -117,7 +119,14 @@
       py_val = (char *) config_values[i];
       svn_config_get(cfg, (const char **) &c_val, "section1", key,
                      "default value");
- if (c_val != NULL && py_val != NULL && strcmp(c_val, py_val) != 0)
+#if 0
+ printf("Testing expected value '%s' against '%s' for "
+ "option '%s'\n", py_val, c_val, key);
+#endif
+ /* Fail iff one value is null, or the strings don't match. */
+ if ((c_val == NULL ^ py_val == NULL)
+ || (c_val != NULL && py_val != NULL
+ && apr_strnatcmp(c_val, py_val) != 0))
         return fail(pool, "Expected value '%s' not equal to '%s' for "
                     "option '%s'", py_val, c_val, key);
     }

-- 
Daniel Rall <dlr@finemaltcoding.com>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org