@matheusbsbs / matheus-bagliano / commits / 9e9de18f1a

config: silence warnings for command names with invalid keys

When we are running the git command "foo", we may have to look up the config keys "pager.foo" and "alias.foo". These config schemes are mis-designed, as the command names can be anything, but the config syntax has some restrictions. For example: $ git foo_bar error: invalid key: pager.foo_bar error: invalid key: alias.foo_bar git: 'foo_bar' is not a git command. See 'git --help'. You cannot name an alias with an underscore. And if you have an external command with one, you cannot configure its pager. In the long run, we may develop a different config scheme for these features. But in the near term (and because we'll need to support the existing scheme indefinitely), we should at least squelch the error messages shown above. These errors come from git_config_parse_key. Ideally we would pass a "quiet" flag to the config machinery, but there are many layers between the pager code and the key parsing. Passing a flag through all of those would be an invasive change. Instead, let's provide a config function to report on whether a key is syntactically valid, and have the pager and alias code skip lookup for bogus keys. We can build this easily around the existing git_config_parse_key, with two minor modifications: 1. We now handle a NULL store_key, to validate but not write out the normalized key. 2. We accept a "quiet" flag to avoid writing to stderr. This doesn't need to be a full-blown public "flags" field, because we can make the existing implementation a static helper function, keeping the mess contained inside config.c. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 24, 2015 at 02:11 UTC 9e9de18f1ad39901a8f0c67f0af70d66d427e326

5 files changed +43 -12

alias.c

+2 -1

@@ -5,7 +5,8 @@ char *alias_lookup(const char *alias)

5 char *v = NULL;

6 struct strbuf key = STRBUF_INIT;

7 strbuf_addf(&key, "alias.%s", alias);

8 - git_config_get_string(key.buf, &v);

8 + if (git_config_key_is_valid(key.buf))

9 + git_config_get_string(key.buf, &v);

10 strbuf_release(&key);

11 return v;

12 }

cache.h

@@ -1366,6 +1366,7 @@ extern int git_config_pathname(const char **, const char *, const char *);

1366 extern int git_config_set_in_file(const char *, const char *, const char *);

1367 extern int git_config_set(const char *, const char *);

1368 extern int git_config_parse_key(const char *, char **, int *);

1369 +extern int git_config_key_is_valid(const char *key);

1370 extern int git_config_set_multivar(const char *, const char *, const char *, int);

1371 extern int git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);

1372 extern int git_config_rename_section(const char *, const char *);

config.c

+29 -10

@@ -1843,7 +1843,7 @@ int git_config_set(const char *key, const char *value)

1843 * baselen - pointer to int which will hold the length of the

1844 * section + subsection part, can be NULL

1845 */

1846 -int git_config_parse_key(const char *key, char **store_key, int *baselen_)

1846 +static int git_config_parse_key_1(const char *key, char **store_key, int *baselen_, int quiet)

1847 {

1848 int i, dot, baselen;

1849 const char *last_dot = strrchr(key, '.');

@@ -1854,12 +1854,14 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_)

1854 */

1855

1856 if (last_dot == NULL || last_dot == key) {

1857 - error("key does not contain a section: %s", key);

1857 + if (!quiet)

1858 + error("key does not contain a section: %s", key);

1859 return -CONFIG_NO_SECTION_OR_NAME;

1860 }

1861

1862 if (!last_dot[1]) {

1862 - error("key does not contain variable name: %s", key);

1863 + if (!quiet)

1864 + error("key does not contain variable name: %s", key);

1865 return -CONFIG_NO_SECTION_OR_NAME;

1866 }

1867

@@ -1870,7 +1872,8 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_)

1872 /*

1873 * Validate the key and while at it, lower case it for matching.

1874 */

1873 - *store_key = xmalloc(strlen(key) + 1);

1875 + if (store_key)

1876 + *store_key = xmalloc(strlen(key) + 1);

1877

1878 dot = 0;

1879 for (i = 0; key[i]; i++) {

@@ -1881,26 +1884,42 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_)

1884 if (!dot || i > baselen) {

1885 if (!iskeychar(c) ||

1886 (i == baselen + 1 && !isalpha(c))) {

1884 - error("invalid key: %s", key);

1887 + if (!quiet)

1888 + error("invalid key: %s", key);

1889 goto out_free_ret_1;

1890 }

1891 c = tolower(c);

1892 } else if (c == '\n') {

1889 - error("invalid key (newline): %s", key);

1893 + if (!quiet)

1894 + error("invalid key (newline): %s", key);

1895 goto out_free_ret_1;

1896 }

1892 - (*store_key)[i] = c;

1897 + if (store_key)

1898 + (*store_key)[i] = c;

1899 }

1894 - (*store_key)[i] = 0;

1900 + if (store_key)

1901 + (*store_key)[i] = 0;

1902

1903 return 0;

1904

1905 out_free_ret_1:

1899 - free(*store_key);

1900 - *store_key = NULL;

1906 + if (store_key) {

1907 + free(*store_key);

1908 + *store_key = NULL;

1909 + }

1910 return -CONFIG_INVALID_KEY;

1911 }

1912

1913 +int git_config_parse_key(const char *key, char **store_key, int *baselen)

1914 +{

1915 + return git_config_parse_key_1(key, store_key, baselen, 0);

1916 +}

1917 +

1918 +int git_config_key_is_valid(const char *key)

1919 +{

1920 + return !git_config_parse_key_1(key, NULL, NULL, 1);

1921 +}

1922 +

1923 /*

1924 * If value==NULL, unset in (remove from) config,

1925 * if value_regex!=NULL, disregard key/value pairs where value does not match.

pager.c

+2 -1

@@ -149,7 +149,8 @@ int check_pager_config(const char *cmd)

149 struct strbuf key = STRBUF_INIT;

150 const char *value = NULL;

151 strbuf_addf(&key, "pager.%s", cmd);

152 - if (!git_config_get_value(key.buf, &value)) {

152 + if (git_config_key_is_valid(key.buf) &&

153 + !git_config_get_value(key.buf, &value)) {

154 int b = git_config_maybe_bool(key.buf, value);

155 if (b >= 0)

156 want = b;

t/t7006-pager.sh

@@ -447,4 +447,13 @@ test_expect_success TTY 'external command pagers override sub-commands' '

447 test_cmp expect actual

448 '

449

450 +test_expect_success 'command with underscores does not complain' '

451 + write_script git-under_score <<-\EOF &&

452 + echo ok

453 + EOF

454 + git --exec-path=. under_score >actual 2>&1 &&

455 + echo ok >expect &&

456 + test_cmp expect actual

457 +'

458 +

459 test_done