[Patch v8]: git_die_config now allows custom error messages.
        new tests are now not too reliant on specific strings. Diff
        between v7 & v8 is appended at the bottom. Thanks to Junio &
        Matthieu for their suggestions.

[Patch v7]: style nit corrected. (1/8) is Matthieu's translation patch.
        git_die_config_linenr() helper function added. Diff between v6
        and v7 appended for review.

[Patch v6]: Added _(....) to error messages.
        Diff between v6 and v4 at the bottom.

[PATCH v5]: New patch added (3/7). git_config() now returns void.

[PATCH v4]: One style nit corrected, also added key to error messages.

[PATCH V3]:All the suggestions in [3] applied. Built on top of [1].

[PATCH V2]: All the suggestions in [2] incorporated. git_config() now follows
        correct parsing order. Reordered the patches. Removed xfuncname patch
        as it was unnecssary.

This series builds on the top of topic[1] in the mailing list with name
"git config cache & special querying API utilizing the cache".

This series aims to do these three things,

* Use the config-set API to rewrite git_config().

* Solve any legacy bugs in the previous system while at it.

* To be feature complete compared to the previous git_config() implementation,
        which I think it is now. (added the line number and file name info just 
for
        completeness)

Also, I haven't yet checked the exact improvements but still as a teaser,
git status now only rereads the configuration files twice instead of four
times.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/254286
[2]: http://thread.gmane.org/gmane.comp.version-control.git/254101
[3]: http://thread.gmane.org/gmane.comp.version-control.git/254211

Matthieu Moy (1):
  config.c: mark error and warnings strings for translation

Tanay Abhra (7):
  config.c: fix accuracy of line number in errors
  add line number and file name info to `config_set`
  change `git_config()` return value to void
  config: add `git_die_config()` to the config-set API
  rewrite git_config() to use the config-set API
  add a test for semantic errors in config files
  add tests for `git_config_get_string_const()`

 Documentation/technical/api-config.txt |  13 +++
 branch.c                               |   5 +-
 cache.h                                |  34 +++++++-
 config.c                               | 152 +++++++++++++++++++++++++++------
 t/t1308-config-set.sh                  |  21 +++++
 t/t4055-diff-context.sh                |   2 +-
 test-config.c                          |  10 +++
 7 files changed, 207 insertions(+), 30 deletions(-)

-- 
1.9.0.GIT

-- 8< --
diff --git a/Documentation/technical/api-config.txt 
b/Documentation/technical/api-config.txt
index d7d0cf9..0d8b99b 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -155,10 +155,11 @@ as well as retrieval for the queried variable, including:
        Similar to `git_config_get_string`, but expands `~` or `~user` into
        the user's home directory when found at the beginning of the path.
 
-`void git_die_config(const char *key)`::
+`git_die_config(const char *key, const char *err, ...)`::
 
-       Dies printing the line number and the file name of the highest
-       priority value for the configuration variable `key`.
+       First prints the error message specified by the caller in `err` and then
+       dies printing the line number and the file name of the highest priority
+       value for the configuration variable `key`.
 
 `void git_die_config_linenr(const char *key, const char *filename, int 
linenr)`::
 
diff --git a/cache.h b/cache.h
index ff17889..2693a37 100644
--- a/cache.h
+++ b/cache.h
@@ -1406,8 +1406,14 @@ extern int git_config_get_bool(const char *key, int 
*dest);
 extern int git_config_get_bool_or_int(const char *key, int *is_bool, int 
*dest);
 extern int git_config_get_maybe_bool(const char *key, int *dest);
 extern int git_config_get_pathname(const char *key, const char **dest);
-extern void git_die_config(const char *key);
-extern void git_die_config_linenr(const char *key, const char *filename, int 
linenr);
+
+struct key_value_info {
+       const char *filename;
+       int linenr;
+};
+
+extern NORETURN void git_die_config(const char *key, const char *err, ...) 
__attribute__((format(printf, 2, 3)));
+extern NORETURN void git_die_config_linenr(const char *key, const char 
*filename, int linenr);
 
 extern int committer_ident_sufficiently_given(void);
 extern int author_ident_sufficiently_given(void);
diff --git a/config.c b/config.c
index cf9124f..427850a 100644
--- a/config.c
+++ b/config.c
@@ -1224,11 +1224,6 @@ int git_config_with_options(config_fn_t fn, void *data,
        return ret;
 }
 
-struct key_value_info {
-       const char *filename;
-       int linenr;
-};
-
 static void git_config_raw(config_fn_t fn, void *data)
 {
        if (git_config_with_options(fn, data, NULL, 1) < 0)
@@ -1326,8 +1321,8 @@ static int configset_add_value(struct config_set *cs, 
const char *key, const cha
                kv_info->linenr = cf->linenr;
        } else {
                /* for values read from `git_config_from_parameters()` */
-               kv_info->filename = strintern("parameter");
-               kv_info->linenr = 0;
+               kv_info->filename = NULL;
+               kv_info->linenr = -1;
        }
        si->util = kv_info;
 
@@ -1513,7 +1508,7 @@ int git_config_get_string_const(const char *key, const 
char **dest)
        git_config_check_init();
        ret = git_configset_get_string_const(&the_config_set, key, dest);
        if (ret < 0)
-               git_die_config(key);
+               git_die_config(key, NULL);
        return ret;
 }
 
@@ -1559,27 +1554,32 @@ int git_config_get_pathname(const char *key, const char 
**dest)
        git_config_check_init();
        ret = git_configset_get_pathname(&the_config_set, key, dest);
        if (ret < 0)
-               git_die_config(key);
+               git_die_config(key, NULL);
        return ret;
 }
 
 NORETURN
 void git_die_config_linenr(const char *key, const char *filename, int linenr)
 {
-       if (!linenr)
+       if (!filename)
                die(_("unable to parse '%s' from command-line config"), key);
        else
-               die(_("bad config variable '%s' at file line %d in %s"),
-                       key,
-                       linenr,
-                       filename);
+               die(_("bad config variable '%s' in file '%s' at line %d"),
+                   key, filename, linenr);
 }
 
-NORETURN
-void git_die_config(const char *key)
+NORETURN __attribute__((format(printf, 2, 3)))
+void git_die_config(const char *key, const char *err, ...)
 {
        const struct string_list *values;
        struct key_value_info *kv_info;
+
+       if (err) {
+               va_list params;
+               va_start(params, err);
+               vreportf("error: ", err, params);
+               va_end(params);
+       }
        values = git_config_get_value_multi(key);
        kv_info = values->items[values->nr - 1].util;
        git_die_config_linenr(key, kv_info->filename, kv_info->linenr);
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
index f012dd6..ecc757a 100755
--- a/t/t1308-config-set.sh
+++ b/t/t1308-config-set.sh
@@ -126,7 +126,7 @@ test_expect_success 'find string value for a key' '
 
 test_expect_success 'check line error when NULL string is queried' '
        test_expect_code 128 test-config get_string case.foo 2>result &&
-       grep "fatal: bad config variable '\''case.foo'\'' at file line 7 in 
.git/config" result
+       grep "line 7.*.git/config\|.git/config.*line 7" result
 '
 
 test_expect_success 'find integer if value is non parse-able' '
@@ -215,7 +215,7 @@ test_expect_success 'check line errors for malformed 
values' '
                br
        EOF
        test_expect_code 128 git br 2>result &&
-       grep "fatal: bad config variable '\''alias.br'\'' at file line 2 in 
.git/config" result
+       grep "line 2.*.git/config\|.git/config.*line 2" result
 '
 
 test_done
-- 8< --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to