Dan, just a minor nit: in addition to the issue number, a summary
(however brief) should lead the log message, especially for a change
like this.
And in the issue itself (1165), your last annotation pasted in the log
message, but didn't give the revision in which it was committed :-).
Better just to give the revision number and let people track down the
details of the change from that if they need to.
-K
dlr@tigris.org writes:
> Log:
> http://subversion.tigris.org/issues/show_bug.cgi?id=1165
>
> * subversion/libsvn_subr/config_impl.h
> (SVN_CONFIG__DEFAULT_SECTION): The name of the default configuration
> section.
>
> * subversion/libsvn_subr/config.c
> (find_option): If the option in the specified section isn't found,
> 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 and
> apr_pool_t pointer for use by expand_option_value()/find_option().
>
> (FMT_START, FMT_END, FMT_START_LEN, FMT_END_LEN): 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. Config text which would be exceptions
> to Python's ConfigParser (such as the failure to resolve an
> expansion, or an unterminated format string) is treated as plain
> text.
>
> (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 * and NULL apr_pool_t *
> to make_string_from_option() to handle the function's API change.
>
> * subversion/tests/libsvn_subr/config-test.c
> Test option 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.
>
>
> Modified: trunk/subversion/libsvn_subr/config.c
> ==============================================================================
> --- trunk/subversion/libsvn_subr/config.c (original)
> +++ trunk/subversion/libsvn_subr/config.c Mon Mar 24 10:24:58 2003
> @@ -361,35 +361,148 @@
> 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, SVN_CONFIG__DEFAULT_SECTION) != 0)
> + /* Options which aren't found in the requested section are
> + also sought after in the default section. */
> + opt = find_option (cfg, SVN_CONFIG__DEFAULT_SECTION, option, &sec);
> + return opt;
> }
>
> return NULL;
> }
>
>
> -/* Set *VALUEP according to the OPT's value. */
> +/* Has a bi-directional dependency with make_string_from_option(). */
> +static void
> +expand_option_value (svn_config_t *cfg, cfg_section_t *section,
> + const char *opt_value, const char **opt_x_valuep,
> + apr_pool_t *x_pool);
> +
> +
> +/* Set *VALUEP according to the OPT's value. A value for X_POOL must
> + only ever be passed into this function by expand_option_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,
> + apr_pool_t* x_pool)
> {
> - /* ### TODO: Expand the option's value */
> - (void)(cfg);
> + apr_pool_t *tmp_pool = (x_pool ? x_pool : svn_pool_create (cfg->x_pool));
> + expand_option_value (cfg, section, opt->value, &opt->x_value, tmp_pool);
> + opt->expanded = TRUE;
>
> /* For legacy reasons, the cfg is still using counted-length strings
> internally. But the public interfaces just use null-terminated
> C strings now, so below we ignore length and use only data. */
>
> if (opt->x_value)
> - *valuep = opt->x_value;
> + {
> + if (!x_pool)
> + /* Grab the fully expanded value from tmp_pool before its
> + disappearing act. */
> + opt->x_value = apr_pstrmemdup (cfg->x_pool, opt->x_value,
> + strlen (opt->x_value));
> + *valuep = opt->x_value;
> + }
> else
> *valuep = opt->value;
> +
> + if (!x_pool)
> + svn_pool_destroy (tmp_pool);
> +}
> +
> +
> +#define FMT_START "%("
> +#define FMT_END ")s"
> +#define FMT_START_LEN (sizeof (FMT_START) - 1)
> +#define FMT_END_LEN (sizeof (FMT_END) - 1)
> +
> +
> +static void
> +expand_option_value (svn_config_t *cfg, cfg_section_t *section,
> + const char *opt_value, const char **opt_x_valuep,
> + apr_pool_t *x_pool)
> +{
> + svn_stringbuf_t *buf = NULL;
> + const char *parse_from = opt_value;
> + const char *copy_from = parse_from;
> + const char *name_start, *name_end;
> +
> + while (parse_from != NULL
> + && *parse_from != '\0'
> + && (name_start = strstr (parse_from, FMT_START)) != NULL)
> + {
> + name_start += FMT_START_LEN;
> + if (*name_start == '\0')
> + /* FMT_START at end of opt_value. */
> + break;
> +
> + name_end = strstr (name_start, FMT_END);
> + if (name_end != NULL)
> + {
> + cfg_option_t *x_opt;
> + apr_size_t len = name_end - name_start;
> + char *name = apr_pstrmemdup (x_pool, name_start, len);
> +
> + x_opt = find_option (cfg, section->name, name, NULL);
> +
> + if (x_opt != NULL)
> + {
> + const char *cstring;
> +
> + /* Pass back the sub-pool originally provided by
> + make_string_from_option() as an indication of when it
> + should terminate. */
> + make_string_from_option (&cstring, cfg, section, x_opt, x_pool);
> +
> + /* Append the plain text preceding the expansion. */
> + len = name_start - FMT_START_LEN - copy_from;
> + if (buf == NULL)
> + {
> + buf = svn_stringbuf_ncreate (copy_from, len, x_pool);
> + cfg->x_values = TRUE;
> + }
> + else
> + svn_stringbuf_appendbytes(buf, copy_from, len);
> +
> + /* Append the expansion and adjust parse pointers. */
> + svn_stringbuf_appendcstr (buf, cstring);
> + parse_from = name_end + FMT_END_LEN;
> + copy_from = parse_from;
> + }
> + else
> + /* Though ConfigParser considers the failure to resolve
> + the requested expansion an exception condition, we
> + consider it to be plain text, and look for the start of
> + the next one. */
> + parse_from = name_end + FMT_END_LEN;
> + }
> + else
> + /* Though ConfigParser treats unterminated format specifiers
> + as an exception condition, we consider them to be plain
> + text. The fact that there are no more format specifier
> + endings means we're done parsing. */
> + parse_from = NULL;
> + }
> +
> + if (buf != NULL)
> + {
> + /* Copy the remainder of the plain text. */
> + svn_stringbuf_appendcstr (buf, copy_from);
> +
> + *opt_x_valuep = apr_pstrmemdup (x_pool, buf->data, buf->len);
> + }
> + else
> + *opt_x_valuep = NULL;
> }
>
>
> @@ -401,11 +514,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, NULL);
> else
> - *valuep = default_value; /* ### TODO: Expand default_value */
> + expand_option_value (cfg, sec, default_value, valuep, cfg->x_pool);
> }
> else
> *valuep = default_value;
> @@ -481,7 +595,7 @@
> opt = opt_ptr;
>
> ++count;
> - make_string_from_option (&temp_value, cfg, opt);
> + make_string_from_option (&temp_value, cfg, sec, opt, NULL);
> if (!callback (opt->name, temp_value, baton))
> break;
> }
>
> Modified: trunk/subversion/libsvn_subr/config_impl.h
> ==============================================================================
> --- trunk/subversion/libsvn_subr/config_impl.h (original)
> +++ trunk/subversion/libsvn_subr/config_impl.h Mon Mar 24 10:24:58 2003
> @@ -61,6 +61,7 @@
> const char *file,
> svn_boolean_t must_exist);
>
> +#define SVN_CONFIG__DEFAULT_SECTION "DEFAULT"
>
> #ifdef SVN_WIN32
> /* Get the common or user-specific AppData folder */
>
> Modified: trunk/subversion/tests/libsvn_subr/config-test.c
> ==============================================================================
> --- trunk/subversion/tests/libsvn_subr/config-test.c (original)
> +++ trunk/subversion/tests/libsvn_subr/config-test.c Mon Mar 24 10:24:58 2003
> @@ -84,9 +84,13 @@
> }
>
>
> -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", "i", NULL };
> +static const char *config_values[] = { "bar", "Aa", "100", "bar",
> + "a %(bogus)s oyster bar",
> + "%(bogus)s shmoo %(",
> + "%Aa", "lyrical bard", "%(unterminated",
> + "Aa 100", NULL };
>
> static svn_error_t *
> test1 (const char **msg,
> @@ -117,7 +121,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);
> }
>
> Modified: trunk/subversion/tests/libsvn_subr/config-test.cfg
> ==============================================================================
> --- trunk/subversion/tests/libsvn_subr/config-test.cfg (original)
> +++ trunk/subversion/tests/libsvn_subr/config-test.cfg Mon Mar 24 10:24:58 2003
> @@ -1,3 +1,25 @@
> +# 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=a %(bogus)s oyster %(foo)s
> +e=%(bogus)s shmoo %(
> +# Expansion format escaping doesn't seem possible
> +f=%%(a)s
> +# Two-level variable expansion with surrounding text
> +g=lyrical %(c)sd
> +# Unterminated format string
> +h= %(unterminated
> +# Multiple expansions
> +i=%(a)s %(b)s
> \ No newline at end of file
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Mar 25 02:38:18 2003