{"thread":{"id":"52672","subject":"[PATCH 0/7] remote rename: improve handling of configuration values","startedAt":"2020-01-21T09:25:00Z","lastAt":"2020-01-24T08:49:23Z","messageCount":18,"participants":["Bert Wesarg","Junio C Hamano","Matt Rogers"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"390151","messageId":"cover.1579598053.git.bert.wesarg@googlemail.com","threadId":"52672","inReplyTo":null,"subject":"[PATCH 0/7] remote rename: improve handling of configuration values","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-21T09:24:48Z","receivedAt":"2020-01-21T09:25:00Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"While fixing that 'git remote rename X Y' does not rename the values for\n'branch.*.pushRemote', it opened the possibility to more improvements in\nthis area:\n\n - 'remote rename' did not accept single-letter abbreviations for\n   'branch.*.rebase' like 'pull --rebase' does\n\n - minor clean-ups the config callback\n\n - patch 5 will be replaced by/rebased on Matthew's work in 'config: allow user to\n   know scope of config options', once 'config_scope_name' is available\n\n - gently handling the rename of 'remote.pushDefault'\n\nBert Wesarg (7):\n  pull --rebase/remote rename: document and honor single-letter\n    abbreviations rebase types\n  remote: clean-up by returning early to avoid one indentation\n  remote: clean-up config callback\n  remote rename: rename branch.<name>.pushRemote config values too\n  [RFC] config: make `scope_name` global as `config_scope_name`\n  config: provide access to the current line number\n  remote rename: gently handle remote.pushDefault config\n\n Documentation/config/branch.txt |   7 +-\n Documentation/config/pull.txt   |   7 +-\n Makefile                        |   1 +\n builtin/pull.c                  |  29 +-----\n builtin/remote.c                | 168 +++++++++++++++++++++-----------\n config.c                        |  24 +++++\n config.h                        |   2 +\n rebase.c                        |  24 +++++\n rebase.h                        |  15 +++\n t/helper/test-config.c          |  18 +---\n t/t1308-config-set.sh           |  14 ++-\n t/t5505-remote.sh               |  52 +++++++++-\n 12 files changed, 254 insertions(+), 107 deletions(-)\n create mode 100644 rebase.c\n create mode 100644 rebase.h\n\n-- \n2.24.1.497.g9abd7b20b4.dirty\n\n"},{"id":"390152","messageId":"59b97032fa158ccc9aee9d52b9cb969cd8df6a5f.1579598053.git.bert.wesarg@googlemail.com","threadId":"52672","inReplyTo":"cover.1579598053.git.bert.wesarg@googlemail.com","subject":"[PATCH 2/7] remote: clean-up by returning early to avoid one indentation","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-21T09:24:50Z","receivedAt":"2020-01-21T09:25:02Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\n---\nCc: Junio C Hamano <gitster@pobox.com>\n---\n builtin/remote.c | 86 +++++++++++++++++++++++++-----------------------\n 1 file changed, 44 insertions(+), 42 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 2830c4ab33..a8bdaca4f4 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -263,50 +263,52 @@ static const char *abbrev_ref(const char *name, const char *prefix)\n \n static int config_read_branches(const char *key, const char *value, void *cb)\n {\n-\tif (starts_with(key, \"branch.\")) {\n-\t\tconst char *orig_key = key;\n-\t\tchar *name;\n-\t\tstruct string_list_item *item;\n-\t\tstruct branch_info *info;\n-\t\tenum { REMOTE, MERGE, REBASE } type;\n-\t\tsize_t key_len;\n-\n-\t\tkey += 7;\n-\t\tif (strip_suffix(key, \".remote\", &key_len)) {\n-\t\t\tname = xmemdupz(key, key_len);\n-\t\t\ttype = REMOTE;\n-\t\t} else if (strip_suffix(key, \".merge\", &key_len)) {\n-\t\t\tname = xmemdupz(key, key_len);\n-\t\t\ttype = MERGE;\n-\t\t} else if (strip_suffix(key, \".rebase\", &key_len)) {\n-\t\t\tname = xmemdupz(key, key_len);\n-\t\t\ttype = REBASE;\n-\t\t} else\n-\t\t\treturn 0;\n+\tif (!starts_with(key, \"branch.\"))\n+\t\treturn 0;\n \n-\t\titem = string_list_insert(&branch_list, name);\n+\tconst char *orig_key = key;\n+\tchar *name;\n+\tstruct string_list_item *item;\n+\tstruct branch_info *info;\n+\tenum { REMOTE, MERGE, REBASE } type;\n+\tsize_t key_len;\n+\n+\tkey += 7;\n+\tif (strip_suffix(key, \".remote\", &key_len)) {\n+\t\tname = xmemdupz(key, key_len);\n+\t\ttype = REMOTE;\n+\t} else if (strip_suffix(key, \".merge\", &key_len)) {\n+\t\tname = xmemdupz(key, key_len);\n+\t\ttype = MERGE;\n+\t} else if (strip_suffix(key, \".rebase\", &key_len)) {\n+\t\tname = xmemdupz(key, key_len);\n+\t\ttype = REBASE;\n+\t} else\n+\t\treturn 0;\n+\n+\titem = string_list_insert(&branch_list, name);\n+\n+\tif (!item->util)\n+\t\titem->util = xcalloc(1, sizeof(struct branch_info));\n+\tinfo = item->util;\n+\tif (type == REMOTE) {\n+\t\tif (info->remote_name)\n+\t\t\twarning(_(\"more than one %s\"), orig_key);\n+\t\tinfo->remote_name = xstrdup(value);\n+\t} else if (type == MERGE) {\n+\t\tchar *space = strchr(value, ' ');\n+\t\tvalue = abbrev_branch(value);\n+\t\twhile (space) {\n+\t\t\tchar *merge;\n+\t\t\tmerge = xstrndup(value, space - value);\n+\t\t\tstring_list_append(&info->merge, merge);\n+\t\t\tvalue = abbrev_branch(space + 1);\n+\t\t\tspace = strchr(value, ' ');\n+\t\t}\n+\t\tstring_list_append(&info->merge, xstrdup(value));\n+\t} else\n+\t\tinfo->rebase = rebase_parse_value(value);\n \n-\t\tif (!item->util)\n-\t\t\titem->util = xcalloc(1, sizeof(struct branch_info));\n-\t\tinfo = item->util;\n-\t\tif (type == REMOTE) {\n-\t\t\tif (info->remote_name)\n-\t\t\t\twarning(_(\"more than one %s\"), orig_key);\n-\t\t\tinfo->remote_name = xstrdup(value);\n-\t\t} else if (type == MERGE) {\n-\t\t\tchar *space = strchr(value, ' ');\n-\t\t\tvalue = abbrev_branch(value);\n-\t\t\twhile (space) {\n-\t\t\t\tchar *merge;\n-\t\t\t\tmerge = xstrndup(value, space - value);\n-\t\t\t\tstring_list_append(&info->merge, merge);\n-\t\t\t\tvalue = abbrev_branch(space + 1);\n-\t\t\t\tspace = strchr(value, ' ');\n-\t\t\t}\n-\t\t\tstring_list_append(&info->merge, xstrdup(value));\n-\t\t} else\n-\t\t\tinfo->rebase = rebase_parse_value(value);\n-\t}\n \treturn 0;\n }\n \n-- \n2.24.1.497.g9abd7b20b4.dirty\n\n"},{"id":"390156","messageId":"f9da9aac7edf6f682592592fe8f450a5801fb012.1579598053.git.bert.wesarg@googlemail.com","threadId":"52672","inReplyTo":"cover.1579598053.git.bert.wesarg@googlemail.com","subject":"[PATCH 1/7] pull --rebase/remote rename: document and honor single-letter abbreviations rebase types","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-21T09:24:49Z","receivedAt":"2020-01-21T09:25:03Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"When 46af44b07d (pull --rebase=<type>: allow single-letter abbreviations\nfor the type, 2018-08-04) landed in Git, it had the side effect that\nnot only 'pull --rebase=<type>' accepted the single-letter abbreviations\nbut also the 'pull.rebase' and 'branch.<name>.rebase' configurations.\n\nSecondly, 'git remote rename' did not honor these single-letter\nabbreviations when reading the 'branch.*.rebase' configurations.\n\nWe now document the single-letter abbreviations and both code places\nshare a common function to parse the values of 'git pull --rebase=*',\n'pull.rebase', and 'branches.*.rebase'.\n\nThe only functional change is the handling of the `branch_info::rebase`\nvalue. Before it was an unsigned enum, thus the truth value could be\nchecked with `branch_info::rebase != 0`. But `enum rebase_type` is\nsigned, thus the truth value must now be checked with\n`branch_info::rebase >= REBASE_TRUE`.\n\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\n---\nCc: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n\nIn case this is considered a BUG, then sharing the function is nevertheless\na good thing. The function could than learn a new flag, indicating whether\nthe single-letter abbreviations are accepted or not.\n---\n Documentation/config/branch.txt |  7 ++++---\n Documentation/config/pull.txt   |  7 ++++---\n Makefile                        |  1 +\n builtin/pull.c                  | 29 ++++-------------------------\n builtin/remote.c                | 26 ++++++++------------------\n rebase.c                        | 24 ++++++++++++++++++++++++\n rebase.h                        | 15 +++++++++++++++\n 7 files changed, 60 insertions(+), 49 deletions(-)\n create mode 100644 rebase.c\n create mode 100644 rebase.h\n\ndiff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt\nindex a592d522a7..cc5f3249fc 100644\n--- a/Documentation/config/branch.txt\n+++ b/Documentation/config/branch.txt\n@@ -81,15 +81,16 @@ branch.<name>.rebase::\n \t\"git pull\" is run. See \"pull.rebase\" for doing this in a non\n \tbranch-specific manner.\n +\n-When `merges`, pass the `--rebase-merges` option to 'git rebase'\n+When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase'\n so that the local merge commits are included in the rebase (see\n linkgit:git-rebase[1] for details).\n +\n-When `preserve` (deprecated in favor of `merges`), also pass\n+When `preserve` (or just 'p', deprecated in favor of `merges`), also pass\n `--preserve-merges` along to 'git rebase' so that locally committed merge\n commits will not be flattened by running 'git pull'.\n +\n-When the value is `interactive`, the rebase is run in interactive mode.\n+When the value is `interactive` (or just 'i'), the rebase is run in interactive\n+mode.\n +\n *NOTE*: this is a possibly dangerous operation; do *not* use\n it unless you understand the implications (see linkgit:git-rebase[1]\ndiff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt\nindex b87cab31b3..5404830609 100644\n--- a/Documentation/config/pull.txt\n+++ b/Documentation/config/pull.txt\n@@ -14,15 +14,16 @@ pull.rebase::\n \tpull\" is run. See \"branch.<name>.rebase\" for setting this on a\n \tper-branch basis.\n +\n-When `merges`, pass the `--rebase-merges` option to 'git rebase'\n+When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase'\n so that the local merge commits are included in the rebase (see\n linkgit:git-rebase[1] for details).\n +\n-When `preserve` (deprecated in favor of `merges`), also pass\n+When `preserve` (or just 'p', deprecated in favor of `merges`), also pass\n `--preserve-merges` along to 'git rebase' so that locally committed merge\n commits will not be flattened by running 'git pull'.\n +\n-When the value is `interactive`, the rebase is run in interactive mode.\n+When the value is `interactive` (or just 'i'), the rebase is run in interactive\n+mode.\n +\n *NOTE*: this is a possibly dangerous operation; do *not* use\n it unless you understand the implications (see linkgit:git-rebase[1]\ndiff --git a/Makefile b/Makefile\nindex 09f98b777c..96ced97bff 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -954,6 +954,7 @@ LIB_OBJS += quote.o\n LIB_OBJS += range-diff.o\n LIB_OBJS += reachable.o\n LIB_OBJS += read-cache.o\n+LIB_OBJS += rebase.o\n LIB_OBJS += rebase-interactive.o\n LIB_OBJS += reflog-walk.o\n LIB_OBJS += refs.o\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex d25ff13a60..888181c07c 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -15,6 +15,7 @@\n #include \"sha1-array.h\"\n #include \"remote.h\"\n #include \"dir.h\"\n+#include \"rebase.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n #include \"revision.h\"\n@@ -26,15 +27,6 @@\n #include \"commit-reach.h\"\n #include \"sequencer.h\"\n \n-enum rebase_type {\n-\tREBASE_INVALID = -1,\n-\tREBASE_FALSE = 0,\n-\tREBASE_TRUE,\n-\tREBASE_PRESERVE,\n-\tREBASE_MERGES,\n-\tREBASE_INTERACTIVE\n-};\n-\n /**\n  * Parses the value of --rebase. If value is a false value, returns\n  * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is\n@@ -45,22 +37,9 @@ enum rebase_type {\n static enum rebase_type parse_config_rebase(const char *key, const char *value,\n \t\tint fatal)\n {\n-\tint v = git_parse_maybe_bool(value);\n-\n-\tif (!v)\n-\t\treturn REBASE_FALSE;\n-\telse if (v > 0)\n-\t\treturn REBASE_TRUE;\n-\telse if (!strcmp(value, \"preserve\") || !strcmp(value, \"p\"))\n-\t\treturn REBASE_PRESERVE;\n-\telse if (!strcmp(value, \"merges\") || !strcmp(value, \"m\"))\n-\t\treturn REBASE_MERGES;\n-\telse if (!strcmp(value, \"interactive\") || !strcmp(value, \"i\"))\n-\t\treturn REBASE_INTERACTIVE;\n-\t/*\n-\t * Please update _git_config() in git-completion.bash when you\n-\t * add new rebase modes.\n-\t */\n+\tenum rebase_type v = rebase_parse_value(value);\n+\tif (v != REBASE_INVALID)\n+\t\treturn v;\n \n \tif (fatal)\n \t\tdie(_(\"Invalid value for %s: %s\"), key, value);\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 96bbe828fe..2830c4ab33 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -6,6 +6,7 @@\n #include \"string-list.h\"\n #include \"strbuf.h\"\n #include \"run-command.h\"\n+#include \"rebase.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n #include \"object-store.h\"\n@@ -248,9 +249,7 @@ static int add(int argc, const char **argv)\n struct branch_info {\n \tchar *remote_name;\n \tstruct string_list merge;\n-\tenum {\n-\t\tNO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE, REBASE_MERGES\n-\t} rebase;\n+\tenum rebase_type rebase;\n };\n \n static struct string_list branch_list = STRING_LIST_INIT_NODUP;\n@@ -305,17 +304,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \t\t\t\tspace = strchr(value, ' ');\n \t\t\t}\n \t\t\tstring_list_append(&info->merge, xstrdup(value));\n-\t\t} else {\n-\t\t\tint v = git_parse_maybe_bool(value);\n-\t\t\tif (v >= 0)\n-\t\t\t\tinfo->rebase = v;\n-\t\t\telse if (!strcmp(value, \"preserve\"))\n-\t\t\t\tinfo->rebase = NORMAL_REBASE;\n-\t\t\telse if (!strcmp(value, \"merges\"))\n-\t\t\t\tinfo->rebase = REBASE_MERGES;\n-\t\t\telse if (!strcmp(value, \"interactive\"))\n-\t\t\t\tinfo->rebase = INTERACTIVE_REBASE;\n-\t\t}\n+\t\t} else\n+\t\t\tinfo->rebase = rebase_parse_value(value);\n \t}\n \treturn 0;\n }\n@@ -943,7 +933,7 @@ static int add_local_to_show_info(struct string_list_item *branch_item, void *cb\n \t\treturn 0;\n \tif ((n = strlen(branch_item->string)) > show_info->width)\n \t\tshow_info->width = n;\n-\tif (branch_info->rebase)\n+\tif (branch_info->rebase >= REBASE_TRUE)\n \t\tshow_info->any_rebase = 1;\n \n \titem = string_list_insert(show_info->list, branch_item->string);\n@@ -960,16 +950,16 @@ static int show_local_info_item(struct string_list_item *item, void *cb_data)\n \tint width = show_info->width + 4;\n \tint i;\n \n-\tif (branch_info->rebase && branch_info->merge.nr > 1) {\n+\tif (branch_info->rebase >= REBASE_TRUE && branch_info->merge.nr > 1) {\n \t\terror(_(\"invalid branch.%s.merge; cannot rebase onto > 1 branch\"),\n \t\t\titem->string);\n \t\treturn 0;\n \t}\n \n \tprintf(\"    %-*s \", show_info->width, item->string);\n-\tif (branch_info->rebase) {\n+\tif (branch_info->rebase >= REBASE_TRUE) {\n \t\tconst char *msg;\n-\t\tif (branch_info->rebase == INTERACTIVE_REBASE)\n+\t\tif (branch_info->rebase == REBASE_INTERACTIVE)\n \t\t\tmsg = _(\"rebases interactively onto remote %s\");\n \t\telse if (branch_info->rebase == REBASE_MERGES)\n \t\t\tmsg = _(\"rebases interactively (with merges) onto \"\ndiff --git a/rebase.c b/rebase.c\nnew file mode 100644\nindex 0000000000..a9ab27205a\n--- /dev/null\n+++ b/rebase.c\n@@ -0,0 +1,24 @@\n+#include \"rebase.h\"\n+#include \"config.h\"\n+\n+enum rebase_type rebase_parse_value(const char *value)\n+{\n+\tint v = git_parse_maybe_bool(value);\n+\n+\tif (!v)\n+\t\treturn REBASE_FALSE;\n+\telse if (v > 0)\n+\t\treturn REBASE_TRUE;\n+\telse if (!strcmp(value, \"preserve\") || !strcmp(value, \"p\"))\n+\t\treturn REBASE_PRESERVE;\n+\telse if (!strcmp(value, \"merges\") || !strcmp(value, \"m\"))\n+\t\treturn REBASE_MERGES;\n+\telse if (!strcmp(value, \"interactive\") || !strcmp(value, \"i\"))\n+\t\treturn REBASE_INTERACTIVE;\n+\t/*\n+\t * Please update _git_config() in git-completion.bash when you\n+\t * add new rebase modes.\n+\t */\n+\n+\treturn REBASE_INVALID;\n+}\ndiff --git a/rebase.h b/rebase.h\nnew file mode 100644\nindex 0000000000..cc723d4748\n--- /dev/null\n+++ b/rebase.h\n@@ -0,0 +1,15 @@\n+#ifndef REBASE_H\n+#define REBASE_H\n+\n+enum rebase_type {\n+\tREBASE_INVALID = -1,\n+\tREBASE_FALSE = 0,\n+\tREBASE_TRUE,\n+\tREBASE_PRESERVE,\n+\tREBASE_MERGES,\n+\tREBASE_INTERACTIVE\n+};\n+\n+enum rebase_type rebase_parse_value(const char *value);\n+\n+#endif /* REBASE */\n-- \n2.24.1.497.g9abd7b20b4.dirty\n\n"},{"id":"390155","messageId":"63df149b8cffb3f4290dee47136e721a5043bd57.1579598053.git.bert.wesarg@googlemail.com","threadId":"52672","inReplyTo":"cover.1579598053.git.bert.wesarg@googlemail.com","subject":"[PATCH 3/7] remote: clean-up config callback","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-21T09:24:51Z","receivedAt":"2020-01-21T09:25:04Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Some minor clean-ups in function `config_read_branches`:\n\n * remove hardcoded length in `key += 7`\n * call `xmemdupz` only once\n * use a switch to handle the configuration type and add a `BUG()`\n\nSuggested-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\n---\nCc: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/remote.c | 28 +++++++++++++++++-----------\n 1 file changed, 17 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex a8bdaca4f4..9466e32b3d 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -273,29 +273,29 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \tenum { REMOTE, MERGE, REBASE } type;\n \tsize_t key_len;\n \n-\tkey += 7;\n-\tif (strip_suffix(key, \".remote\", &key_len)) {\n-\t\tname = xmemdupz(key, key_len);\n+\tkey += strlen(\"branch.\");\n+\tif (strip_suffix(key, \".remote\", &key_len))\n \t\ttype = REMOTE;\n-\t} else if (strip_suffix(key, \".merge\", &key_len)) {\n-\t\tname = xmemdupz(key, key_len);\n+\telse if (strip_suffix(key, \".merge\", &key_len))\n \t\ttype = MERGE;\n-\t} else if (strip_suffix(key, \".rebase\", &key_len)) {\n-\t\tname = xmemdupz(key, key_len);\n+\telse if (strip_suffix(key, \".rebase\", &key_len))\n \t\ttype = REBASE;\n-\t} else\n+\telse\n \t\treturn 0;\n+\tname = xmemdupz(key, key_len);\n \n \titem = string_list_insert(&branch_list, name);\n \n \tif (!item->util)\n \t\titem->util = xcalloc(1, sizeof(struct branch_info));\n \tinfo = item->util;\n-\tif (type == REMOTE) {\n+\tswitch (type) {\n+\tcase REMOTE:\n \t\tif (info->remote_name)\n \t\t\twarning(_(\"more than one %s\"), orig_key);\n \t\tinfo->remote_name = xstrdup(value);\n-\t} else if (type == MERGE) {\n+\t\tbreak;\n+\tcase MERGE: {\n \t\tchar *space = strchr(value, ' ');\n \t\tvalue = abbrev_branch(value);\n \t\twhile (space) {\n@@ -306,8 +306,14 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \t\t\tspace = strchr(value, ' ');\n \t\t}\n \t\tstring_list_append(&info->merge, xstrdup(value));\n-\t} else\n+\t\tbreak;\n+\t}\n+\tcase REBASE:\n \t\tinfo->rebase = rebase_parse_value(value);\n+\t\tbreak;\n+\tdefault:\n+\t\tBUG(\"unexpected type=%d\", type);\n+\t}\n \n \treturn 0;\n }\n-- \n2.24.1.497.g9abd7b20b4.dirty\n\n"},{"id":"390153","messageId":"04eb98389880c96e1dc18131031e9d6ad5830a40.1579598053.git.bert.wesarg@googlemail.com","threadId":"52672","inReplyTo":"cover.1579598053.git.bert.wesarg@googlemail.com","subject":"[PATCH 5/7] [RFC] config: make `scope_name` global as `config_scope_name`","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-21T09:24:53Z","receivedAt":"2020-01-21T09:25:05Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n---\nWill be replaced by Matthew Rogers.\n\nCc: Matthew Rogers <mattr94@gmail.com>\n---\n config.c               | 16 ++++++++++++++++\n config.h               |  1 +\n t/helper/test-config.c | 17 +----------------\n 3 files changed, 18 insertions(+), 16 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex d75f88ca0c..4c461bb7a3 100644\n--- a/config.c\n+++ b/config.c\n@@ -3317,6 +3317,22 @@ enum config_scope current_config_scope(void)\n \t\treturn current_parsing_scope;\n }\n \n+const char *config_scope_name(enum config_scope scope)\n+{\n+\tswitch (scope) {\n+\tcase CONFIG_SCOPE_SYSTEM:\n+\t\treturn \"system\";\n+\tcase CONFIG_SCOPE_GLOBAL:\n+\t\treturn \"global\";\n+\tcase CONFIG_SCOPE_REPO:\n+\t\treturn \"repo\";\n+\tcase CONFIG_SCOPE_CMDLINE:\n+\t\treturn \"cmdline\";\n+\tdefault:\n+\t\treturn \"unknown\";\n+\t}\n+}\n+\n int lookup_config(const char **mapping, int nr_mapping, const char *var)\n {\n \tint i;\ndiff --git a/config.h b/config.h\nindex 91fd4c5e96..c063f33ff6 100644\n--- a/config.h\n+++ b/config.h\n@@ -301,6 +301,7 @@ enum config_scope {\n \tCONFIG_SCOPE_REPO,\n \tCONFIG_SCOPE_CMDLINE,\n };\n+const char *config_scope_name(enum config_scope scope);\n \n enum config_scope current_config_scope(void);\n const char *current_config_origin_type(void);\ndiff --git a/t/helper/test-config.c b/t/helper/test-config.c\nindex 214003d5b2..1e3bc7c8f4 100644\n--- a/t/helper/test-config.c\n+++ b/t/helper/test-config.c\n@@ -37,21 +37,6 @@\n  *\n  */\n \n-static const char *scope_name(enum config_scope scope)\n-{\n-\tswitch (scope) {\n-\tcase CONFIG_SCOPE_SYSTEM:\n-\t\treturn \"system\";\n-\tcase CONFIG_SCOPE_GLOBAL:\n-\t\treturn \"global\";\n-\tcase CONFIG_SCOPE_REPO:\n-\t\treturn \"repo\";\n-\tcase CONFIG_SCOPE_CMDLINE:\n-\t\treturn \"cmdline\";\n-\tdefault:\n-\t\treturn \"unknown\";\n-\t}\n-}\n static int iterate_cb(const char *var, const char *value, void *data)\n {\n \tstatic int nr;\n@@ -63,7 +48,7 @@ static int iterate_cb(const char *var, const char *value, void *data)\n \tprintf(\"value=%s\\n\", value ? value : \"(null)\");\n \tprintf(\"origin=%s\\n\", current_config_origin_type());\n \tprintf(\"name=%s\\n\", current_config_name());\n-\tprintf(\"scope=%s\\n\", scope_name(current_config_scope()));\n+\tprintf(\"scope=%s\\n\", config_scope_name(current_config_scope()));\n \n \treturn 0;\n }\n-- \n2.24.1.497.g9abd7b20b4.dirty\n\n"},{"id":"390154","messageId":"686540e5cd8fa50f841af12425421bac4922ae5f.1579598053.git.bert.wesarg@googlemail.com","threadId":"52672","inReplyTo":"cover.1579598053.git.bert.wesarg@googlemail.com","subject":"[PATCH v3 4/7] remote rename: rename branch.<name>.pushRemote config values too","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-21T09:24:52Z","receivedAt":"2020-01-21T09:25:05Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"When renaming a remote with\n\n    git remote rename X Y\n\nGit already renames any config values from\n\n    branch.<name>.remote = X\n\nto\n\n    branch.<name>.remote = Y\n\nAs branch.<name>.pushRemote also names a remote, it now also renames\nthese config values from\n\n    branch.<name>.pushRemote = X\n\nto\n\n    branch.<name>.pushRemote = Y\n\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\n---\nCc: Junio C Hamano <gitster@pobox.com>\nCc: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/remote.c  | 15 ++++++++++++++-\n t/t5505-remote.sh |  4 +++-\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 9466e32b3d..0cb930fe00 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -250,6 +250,7 @@ struct branch_info {\n \tchar *remote_name;\n \tstruct string_list merge;\n \tenum rebase_type rebase;\n+\tchar *push_remote_name;\n };\n \n static struct string_list branch_list = STRING_LIST_INIT_NODUP;\n@@ -270,7 +271,7 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \tchar *name;\n \tstruct string_list_item *item;\n \tstruct branch_info *info;\n-\tenum { REMOTE, MERGE, REBASE } type;\n+\tenum { REMOTE, MERGE, REBASE, PUSH_REMOTE } type;\n \tsize_t key_len;\n \n \tkey += strlen(\"branch.\");\n@@ -280,6 +281,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \t\ttype = MERGE;\n \telse if (strip_suffix(key, \".rebase\", &key_len))\n \t\ttype = REBASE;\n+\tif (strip_suffix(key, \".pushremote\", &key_len))\n+\t\ttype = PUSH_REMOTE;\n \telse\n \t\treturn 0;\n \tname = xmemdupz(key, key_len);\n@@ -311,6 +314,11 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n \tcase REBASE:\n \t\tinfo->rebase = rebase_parse_value(value);\n \t\tbreak;\n+\tcase PUSH_REMOTE:\n+\t\tif (info->push_remote_name)\n+\t\t\twarning(_(\"more than one %s\"), orig_key);\n+\t\tinfo->push_remote_name = xstrdup(value);\n+\t\tbreak;\n \tdefault:\n \t\tBUG(\"unexpected type=%d\", type);\n \t}\n@@ -678,6 +686,11 @@ static int mv(int argc, const char **argv)\n \t\t\tstrbuf_addf(&buf, \"branch.%s.remote\", item->string);\n \t\t\tgit_config_set(buf.buf, rename.new_name);\n \t\t}\n+\t\tif (info->push_remote_name && !strcmp(info->push_remote_name, rename.old_name)) {\n+\t\t\tstrbuf_reset(&buf);\n+\t\t\tstrbuf_addf(&buf, \"branch.%s.pushremote\", item->string);\n+\t\t\tgit_config_set(buf.buf, rename.new_name);\n+\t\t}\n \t}\n \n \tif (!refspec_updated)\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 883b32efa0..59a1681636 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -737,12 +737,14 @@ test_expect_success 'rename a remote' '\n \tgit clone one four &&\n \t(\n \t\tcd four &&\n+\t\tgit config branch.master.pushRemote origin &&\n \t\tgit remote rename origin upstream &&\n \t\ttest -z \"$(git for-each-ref refs/remotes/origin)\" &&\n \t\ttest \"$(git symbolic-ref refs/remotes/upstream/HEAD)\" = \"refs/remotes/upstream/master\" &&\n \t\ttest \"$(git rev-parse upstream/master)\" = \"$(git rev-parse master)\" &&\n \t\ttest \"$(git config remote.upstream.fetch)\" = \"+refs/heads/*:refs/remotes/upstream/*\" &&\n-\t\ttest \"$(git config branch.master.remote)\" = \"upstream\"\n+\t\ttest \"$(git config branch.master.remote)\" = \"upstream\" &&\n+\t\ttest \"$(git config branch.master.pushRemote)\" = \"upstream\"\n \t)\n '\n \n-- \n2.24.1.497.g9abd7b20b4.dirty\n\n"},{"id":"390157","messageId":"d10d3049ce9824f6925dddeb12cc130627a8c478.1579598053.git.bert.wesarg@googlemail.com","threadId":"52672","inReplyTo":"cover.1579598053.git.bert.wesarg@googlemail.com","subject":"[PATCH 7/7] remote rename: gently handle remote.pushDefault config","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-21T09:24:55Z","receivedAt":"2020-01-21T09:25:10Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"When renaming a remote with\n\n    git remote rename X Y\n\nGit already renames any branch.<name>.remote and branch.<name>.pushRemote\nvalues from X to Y.\n\nHowever remote.pushDefault needs a more gentle approach, as this may be\nset in a non-repo configuration file. In such a case only a warning is\nprinted, such as:\n\nwarning: The global configuration remote.pushDefault in:\n\t$HOME/.gitconfig:35\nnow names the non-existent remote 'origin'\n\nIt is changed to remote.pushDefault = Y when set in a repo configuration\nthough.\n\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n---\n\nMatthew, you are in Cc because of your current work 'config: allow user to\nknow scope of config options'. I think I'm correct to assuming an ordering\nof the enum config_scope.\n\nCc: Junio C Hamano <gitster@pobox.com>\nCc: Johannes Schindelin <johannes.schindelin@gmx.de>\nCc: Matthew Rogers <mattr94@gmail.com>\n---\n builtin/remote.c  | 43 ++++++++++++++++++++++++++++++++++++++++\n t/t5505-remote.sh | 50 ++++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 92 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 0cb930fe00..52172e523a 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -611,6 +611,29 @@ static int migrate_file(struct remote *remote)\n \treturn 0;\n }\n \n+struct push_default_info\n+{\n+\tstruct rename_info *rename;\n+\tenum config_scope scope;\n+\tstruct strbuf* origin;\n+\tint linenr;\n+};\n+\n+static int config_read_push_default(const char *key, const char *value,\n+\tvoid *cb)\n+{\n+\tstruct push_default_info* info = cb;\n+\tif (strcmp(key, \"remote.pushdefault\") || strcmp(value, info->rename->old_name))\n+\t\treturn 0;\n+\n+\tinfo->scope = current_config_scope();\n+\tstrbuf_reset(info->origin);\n+\tstrbuf_addstr(info->origin, current_config_name());\n+\tinfo->linenr = current_config_line();\n+\n+\treturn 0;\n+}\n+\n static int mv(int argc, const char **argv)\n {\n \tstruct option options[] = {\n@@ -746,6 +769,26 @@ static int mv(int argc, const char **argv)\n \t\t\tdie(_(\"creating '%s' failed\"), buf.buf);\n \t}\n \tstring_list_clear(&remote_branches, 1);\n+\n+\tstruct push_default_info push_default;\n+\tpush_default.rename = &rename;\n+\tpush_default.scope = CONFIG_SCOPE_UNKNOWN;\n+\tpush_default.origin = &buf;\n+\tgit_config(config_read_push_default, &push_default);\n+\tif (push_default.scope >= CONFIG_SCOPE_CMDLINE)\n+\t\t; /* pass */\n+\telse if (push_default.scope >= CONFIG_SCOPE_REPO) {\n+\t\tgit_config_set(\"remote.pushDefault\", rename.new_name);\n+\t} else if (push_default.scope >= CONFIG_SCOPE_SYSTEM) {\n+\t\t/* warn */\n+\t\twarning(_(\"The %s configuration remote.pushDefault in:\\n\"\n+\t\t\t  \"\\t%s:%d\\n\"\n+\t\t\t  \"now names the non-existent remote '%s'\"),\n+\t\t\tconfig_scope_name(push_default.scope),\n+\t\t\tpush_default.origin->buf, push_default.linenr,\n+\t\t\trename.old_name);\n+\t}\n+\n \treturn 0;\n }\n \ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 59a1681636..3b84c7bf9b 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -737,6 +737,7 @@ test_expect_success 'rename a remote' '\n \tgit clone one four &&\n \t(\n \t\tcd four &&\n+\t\ttest_config_global remote.pushDefault origin &&\n \t\tgit config branch.master.pushRemote origin &&\n \t\tgit remote rename origin upstream &&\n \t\ttest -z \"$(git for-each-ref refs/remotes/origin)\" &&\n@@ -744,7 +745,54 @@ test_expect_success 'rename a remote' '\n \t\ttest \"$(git rev-parse upstream/master)\" = \"$(git rev-parse master)\" &&\n \t\ttest \"$(git config remote.upstream.fetch)\" = \"+refs/heads/*:refs/remotes/upstream/*\" &&\n \t\ttest \"$(git config branch.master.remote)\" = \"upstream\" &&\n-\t\ttest \"$(git config branch.master.pushRemote)\" = \"upstream\"\n+\t\ttest \"$(git config branch.master.pushRemote)\" = \"upstream\" &&\n+\t\ttest \"$(git config --global remote.pushDefault)\" = \"origin\"\n+\t)\n+'\n+\n+test_expect_success 'rename a remote renames repo remote.pushDefault' '\n+\tgit clone one four.1 &&\n+\t(\n+\t\tcd four.1 &&\n+\t\tgit config remote.pushDefault origin &&\n+\t\tgit remote rename origin upstream &&\n+\t\ttest \"$(git config remote.pushDefault)\" = \"upstream\"\n+\t)\n+'\n+\n+test_expect_success 'rename a remote keeps global remote.pushDefault' '\n+\tgit clone one four.2 &&\n+\t(\n+\t\tcd four.2 &&\n+\t\ttest_config_global remote.pushDefault origin &&\n+\t\tgit config remote.pushDefault other &&\n+\t\tgit remote rename origin upstream &&\n+\t\ttest \"$(git config --global remote.pushDefault)\" = \"origin\" &&\n+\t\ttest \"$(git config remote.pushDefault)\" = \"other\"\n+\t)\n+'\n+\n+test_expect_success 'rename a remote renames repo remote.pushDefault but ignores global' '\n+\tgit clone one four.3 &&\n+\t(\n+\t\tcd four.3 &&\n+\t\ttest_config_global remote.pushDefault other &&\n+\t\tgit config remote.pushDefault origin &&\n+\t\tgit remote rename origin upstream &&\n+\t\ttest \"$(git config --global remote.pushDefault)\" = \"other\" &&\n+\t\ttest \"$(git config remote.pushDefault)\" = \"upstream\"\n+\t)\n+'\n+\n+test_expect_success 'rename a remote renames repo remote.pushDefault but keeps global' '\n+\tgit clone one four.4 &&\n+\t(\n+\t\tcd four.4 &&\n+\t\ttest_config_global remote.pushDefault origin &&\n+\t\tgit config remote.pushDefault origin &&\n+\t\tgit remote rename origin upstream &&\n+\t\ttest \"$(git config --global remote.pushDefault)\" = \"origin\" &&\n+\t\ttest \"$(git config remote.pushDefault)\" = \"upstream\"\n \t)\n '\n \n-- \n2.24.1.497.g9abd7b20b4.dirty\n\n"},{"id":"390158","messageId":"92356342164523c7753eda52c8985cb4774d1434.1579598053.git.bert.wesarg@googlemail.com","threadId":"52672","inReplyTo":"cover.1579598053.git.bert.wesarg@googlemail.com","subject":"[PATCH 6/7] config: provide access to the current line number","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-21T09:24:54Z","receivedAt":"2020-01-21T09:25:10Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Users are nowadays trained to see message from CLI tools in the form\n\n    <file>:<lno>: …\n\nTo be able to give such messages when notifying the user about\nconfigurations in any config file, it is currently only possible to get\nthe file name (if the value originates from a file to begin with) via\n`current_config_name()`. Now it is also possible to query the current line\nnumber for the configuration.\n\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n---\n config.c               |  8 ++++++++\n config.h               |  1 +\n t/helper/test-config.c |  1 +\n t/t1308-config-set.sh  | 14 ++++++++++++--\n 4 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 4c461bb7a3..5d1d6b5871 100644\n--- a/config.c\n+++ b/config.c\n@@ -3333,6 +3333,14 @@ const char *config_scope_name(enum config_scope scope)\n \t}\n }\n \n+int current_config_line(void)\n+{\n+\tif (current_config_kvi)\n+\t\treturn current_config_kvi->linenr;\n+\telse\n+\t\treturn cf->linenr;\n+}\n+\n int lookup_config(const char **mapping, int nr_mapping, const char *var)\n {\n \tint i;\ndiff --git a/config.h b/config.h\nindex c063f33ff6..371f7f2dd0 100644\n--- a/config.h\n+++ b/config.h\n@@ -306,6 +306,7 @@ const char *config_scope_name(enum config_scope scope);\n enum config_scope current_config_scope(void);\n const char *current_config_origin_type(void);\n const char *current_config_name(void);\n+int current_config_line(void);\n \n /**\n  * Include Directives\ndiff --git a/t/helper/test-config.c b/t/helper/test-config.c\nindex 1e3bc7c8f4..234c722b48 100644\n--- a/t/helper/test-config.c\n+++ b/t/helper/test-config.c\n@@ -48,6 +48,7 @@ static int iterate_cb(const char *var, const char *value, void *data)\n \tprintf(\"value=%s\\n\", value ? value : \"(null)\");\n \tprintf(\"origin=%s\\n\", current_config_origin_type());\n \tprintf(\"name=%s\\n\", current_config_name());\n+\tprintf(\"lno=%d\\n\", current_config_line());\n \tprintf(\"scope=%s\\n\", config_scope_name(current_config_scope()));\n \n \treturn 0;\ndiff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\nindex 7b4e1a63eb..9e36e7a590 100755\n--- a/t/t1308-config-set.sh\n+++ b/t/t1308-config-set.sh\n@@ -238,8 +238,8 @@ test_expect_success 'error on modifying repo config without repo' '\n \n cmdline_config=\"'foo.bar=from-cmdline'\"\n test_expect_success 'iteration shows correct origins' '\n-\techo \"[foo]bar = from-repo\" >.git/config &&\n-\techo \"[foo]bar = from-home\" >.gitconfig &&\n+\tprintf \"[ignore]\\n\\tthis = please\\n[foo]bar = from-repo\\n\" >.git/config &&\n+\tprintf \"[foo]\\n\\tbar = from-home\\n\" >.gitconfig &&\n \tif test_have_prereq MINGW\n \tthen\n \t\t# Use Windows path (i.e. *not* $HOME)\n@@ -253,18 +253,28 @@ test_expect_success 'iteration shows correct origins' '\n \tvalue=from-home\n \torigin=file\n \tname=$HOME_GITCONFIG\n+\tlno=2\n \tscope=global\n \n+\tkey=ignore.this\n+\tvalue=please\n+\torigin=file\n+\tname=.git/config\n+\tlno=2\n+\tscope=repo\n+\n \tkey=foo.bar\n \tvalue=from-repo\n \torigin=file\n \tname=.git/config\n+\tlno=3\n \tscope=repo\n \n \tkey=foo.bar\n \tvalue=from-cmdline\n \torigin=command line\n \tname=\n+\tlno=-1\n \tscope=cmdline\n \tEOF\n \tGIT_CONFIG_PARAMETERS=$cmdline_config test-tool config iterate >actual &&\n-- \n2.24.1.497.g9abd7b20b4.dirty\n\n"},{"id":"390214","messageId":"xmqqv9p475ns.fsf@gitster-ct.c.googlers.com","threadId":"52672","inReplyTo":"f9da9aac7edf6f682592592fe8f450a5801fb012.1579598053.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH 1/7] pull --rebase/remote rename: document and honor single-letter abbreviations rebase types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-21T23:26:15Z","receivedAt":"2020-01-21T23:26:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg <bert.wesarg@googlemail.com> writes:\n\n> When 46af44b07d (pull --rebase=<type>: allow single-letter abbreviations\n> for the type, 2018-08-04) landed in Git, it had the side effect that\n> not only 'pull --rebase=<type>' accepted the single-letter abbreviations\n> but also the 'pull.rebase' and 'branch.<name>.rebase' configurations.\n>\n> Secondly, 'git remote rename' did not honor these single-letter\n> abbreviations when reading the 'branch.*.rebase' configurations.\n\nHmph, do you mean s/Secondly/However/ instead?\n\n> The only functional change is the handling of the `branch_info::rebase`\n> value. Before it was an unsigned enum, thus the truth value could be\n> checked with `branch_info::rebase != 0`. But `enum rebase_type` is\n> signed, thus the truth value must now be checked with\n> `branch_info::rebase >= REBASE_TRUE`.\n\nI think there is another hidden one, but I do not know offhand the\nimplications of the change.  It could well be benign.\n\n>  /**\n>   * Parses the value of --rebase. If value is a false value, returns\n>   * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is\n> @@ -45,22 +37,9 @@ enum rebase_type {\n>  static enum rebase_type parse_config_rebase(const char *key, const char *value,\n>  \t\tint fatal)\n>  {\n> -\tint v = git_parse_maybe_bool(value);\n> -\n> -\tif (!v)\n> -\t\treturn REBASE_FALSE;\n> -\telse if (v > 0)\n> -\t\treturn REBASE_TRUE;\n> -\telse if (!strcmp(value, \"preserve\") || !strcmp(value, \"p\"))\n> -\t\treturn REBASE_PRESERVE;\n> -\telse if (!strcmp(value, \"merges\") || !strcmp(value, \"m\"))\n> -\t\treturn REBASE_MERGES;\n> -\telse if (!strcmp(value, \"interactive\") || !strcmp(value, \"i\"))\n> -\t\treturn REBASE_INTERACTIVE;\n> -\t/*\n> -\t * Please update _git_config() in git-completion.bash when you\n> -\t * add new rebase modes.\n> -\t */\n\nI see all of the above, including the \"Please update\" comment, has\nbecome rebase_parse_value(), which is very good.\n\n> diff --git a/builtin/remote.c b/builtin/remote.c\n> index 96bbe828fe..2830c4ab33 100644\n> --- a/builtin/remote.c\n> +++ b/builtin/remote.c\n> @@ -6,6 +6,7 @@\n> ...\n> -\tenum {\n> -\t\tNO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE, REBASE_MERGES\n> -\t} rebase;\n> +\tenum rebase_type rebase;\n\nGood to see the duplicate go.\n\n> @@ -305,17 +304,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n>  \t\t\t\tspace = strchr(value, ' ');\n>  \t\t\t}\n>  \t\t\tstring_list_append(&info->merge, xstrdup(value));\n> -\t\t} else {\n> -\t\t\tint v = git_parse_maybe_bool(value);\n> -\t\t\tif (v >= 0)\n> -\t\t\t\tinfo->rebase = v;\n> -\t\t\telse if (!strcmp(value, \"preserve\"))\n> -\t\t\t\tinfo->rebase = NORMAL_REBASE;\n> -\t\t\telse if (!strcmp(value, \"merges\"))\n> -\t\t\t\tinfo->rebase = REBASE_MERGES;\n> -\t\t\telse if (!strcmp(value, \"interactive\"))\n> -\t\t\t\tinfo->rebase = INTERACTIVE_REBASE;\n> -\t\t}\n> +\t\t} else\n> +\t\t\tinfo->rebase = rebase_parse_value(value);\n\nHere, we never had info->rebase == REBASE_INVALID.  The field was\nleft intact when the configuration file had a rebase type that is\nnot known to this version of git.  Now it has become possible that\ninfo->rebase to be REBASE_INVALID.  Would the code after this part\nreturns be prepared to handle it, and if so how?  At least I think\nit deserves a comment here, or in rebase_parse_value(), to say (1)\nthat unknown rebase value is treated as false for most of the code\nthat do not need to differentiate between false and unknown, and (2)\nthat assigning a negative value to REBASE_INVALID and always\nchecking if the value is the same or greater than REBASE_TRUE helps\nto maintain the convention.\n\n\n> diff --git a/rebase.h b/rebase.h\n> new file mode 100644\n> index 0000000000..cc723d4748\n> --- /dev/null\n> +++ b/rebase.h\n> @@ -0,0 +1,15 @@\n> +#ifndef REBASE_H\n> +#define REBASE_H\n> +\n> +enum rebase_type {\n> +\tREBASE_INVALID = -1,\n> +\tREBASE_FALSE = 0,\n> +\tREBASE_TRUE,\n> +\tREBASE_PRESERVE,\n> +\tREBASE_MERGES,\n> +\tREBASE_INTERACTIVE\n> +};\n> +\n> +enum rebase_type rebase_parse_value(const char *value);\n> +\n> +#endif /* REBASE */\n"},{"id":"390216","messageId":"CAOjrSZsuPUc7kDPh6wTDMq10b2QM0R2Uq7-0TQ=W76yjk-eoJA@mail.gmail.com","threadId":"52672","inReplyTo":"04eb98389880c96e1dc18131031e9d6ad5830a40.1579598053.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH 5/7] [RFC] config: make `scope_name` global as `config_scope_name`","fromName":"Matt Rogers","fromEmail":"mattr94@gmail.com","sentAt":"2020-01-22T00:12:43Z","receivedAt":"2020-01-22T00:12:58Z","isPatch":true,"sender":{"key":"mattr94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5719846?v=4"},"body":"Logos good to me...\n\nAs I'm a bit new, what would be the best way for me to work this into\nmy workflow?\n\nOn Tue, Jan 21, 2020 at 4:25 AM Bert Wesarg <bert.wesarg@googlemail.com> wrote:\n>\n> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n> ---\n> Will be replaced by Matthew Rogers.\n>\n> Cc: Matthew Rogers <mattr94@gmail.com>\n> ---\n>  config.c               | 16 ++++++++++++++++\n>  config.h               |  1 +\n>  t/helper/test-config.c | 17 +----------------\n>  3 files changed, 18 insertions(+), 16 deletions(-)\n>\n> diff --git a/config.c b/config.c\n> index d75f88ca0c..4c461bb7a3 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -3317,6 +3317,22 @@ enum config_scope current_config_scope(void)\n>                 return current_parsing_scope;\n>  }\n>\n> +const char *config_scope_name(enum config_scope scope)\n> +{\n> +       switch (scope) {\n> +       case CONFIG_SCOPE_SYSTEM:\n> +               return \"system\";\n> +       case CONFIG_SCOPE_GLOBAL:\n> +               return \"global\";\n> +       case CONFIG_SCOPE_REPO:\n> +               return \"repo\";\n> +       case CONFIG_SCOPE_CMDLINE:\n> +               return \"cmdline\";\n> +       default:\n> +               return \"unknown\";\n> +       }\n> +}\n> +\n>  int lookup_config(const char **mapping, int nr_mapping, const char *var)\n>  {\n>         int i;\n> diff --git a/config.h b/config.h\n> index 91fd4c5e96..c063f33ff6 100644\n> --- a/config.h\n> +++ b/config.h\n> @@ -301,6 +301,7 @@ enum config_scope {\n>         CONFIG_SCOPE_REPO,\n>         CONFIG_SCOPE_CMDLINE,\n>  };\n> +const char *config_scope_name(enum config_scope scope);\n>\n>  enum config_scope current_config_scope(void);\n>  const char *current_config_origin_type(void);\n> diff --git a/t/helper/test-config.c b/t/helper/test-config.c\n> index 214003d5b2..1e3bc7c8f4 100644\n> --- a/t/helper/test-config.c\n> +++ b/t/helper/test-config.c\n> @@ -37,21 +37,6 @@\n>   *\n>   */\n>\n> -static const char *scope_name(enum config_scope scope)\n> -{\n> -       switch (scope) {\n> -       case CONFIG_SCOPE_SYSTEM:\n> -               return \"system\";\n> -       case CONFIG_SCOPE_GLOBAL:\n> -               return \"global\";\n> -       case CONFIG_SCOPE_REPO:\n> -               return \"repo\";\n> -       case CONFIG_SCOPE_CMDLINE:\n> -               return \"cmdline\";\n> -       default:\n> -               return \"unknown\";\n> -       }\n> -}\n>  static int iterate_cb(const char *var, const char *value, void *data)\n>  {\n>         static int nr;\n> @@ -63,7 +48,7 @@ static int iterate_cb(const char *var, const char *value, void *data)\n>         printf(\"value=%s\\n\", value ? value : \"(null)\");\n>         printf(\"origin=%s\\n\", current_config_origin_type());\n>         printf(\"name=%s\\n\", current_config_name());\n> -       printf(\"scope=%s\\n\", scope_name(current_config_scope()));\n> +       printf(\"scope=%s\\n\", config_scope_name(current_config_scope()));\n>\n>         return 0;\n>  }\n> --\n> 2.24.1.497.g9abd7b20b4.dirty\n>\n\n\n-- \nMatthew Rogers\n"},{"id":"390228","messageId":"CAKPyHN0_AVOo_6bdHvy_J9ebnBpSD2NECBiLZ7g=4TcMvfZgYw@mail.gmail.com","threadId":"52672","inReplyTo":"xmqqv9p475ns.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/7] pull --rebase/remote rename: document and honor single-letter abbreviations rebase types","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-22T07:34:46Z","receivedAt":"2020-01-22T07:35:02Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Dear Junio,\n\nOn Wed, Jan 22, 2020 at 12:26 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Bert Wesarg <bert.wesarg@googlemail.com> writes:\n>\n> > When 46af44b07d (pull --rebase=<type>: allow single-letter abbreviations\n> > for the type, 2018-08-04) landed in Git, it had the side effect that\n> > not only 'pull --rebase=<type>' accepted the single-letter abbreviations\n> > but also the 'pull.rebase' and 'branch.<name>.rebase' configurations.\n> >\n> > Secondly, 'git remote rename' did not honor these single-letter\n> > abbreviations when reading the 'branch.*.rebase' configurations.\n>\n> Hmph, do you mean s/Secondly/However/ instead?\n\nthanks, that now reads smoothly.\n\n> > @@ -305,17 +304,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n> >                               space = strchr(value, ' ');\n> >                       }\n> >                       string_list_append(&info->merge, xstrdup(value));\n> > -             } else {\n> > -                     int v = git_parse_maybe_bool(value);\n> > -                     if (v >= 0)\n> > -                             info->rebase = v;\n> > -                     else if (!strcmp(value, \"preserve\"))\n> > -                             info->rebase = NORMAL_REBASE;\n> > -                     else if (!strcmp(value, \"merges\"))\n> > -                             info->rebase = REBASE_MERGES;\n> > -                     else if (!strcmp(value, \"interactive\"))\n> > -                             info->rebase = INTERACTIVE_REBASE;\n> > -             }\n> > +             } else\n> > +                     info->rebase = rebase_parse_value(value);\n>\n> Here, we never had info->rebase == REBASE_INVALID.  The field was\n> left intact when the configuration file had a rebase type that is\n> not known to this version of git.  Now it has become possible that\n> info->rebase to be REBASE_INVALID.  Would the code after this part\n> returns be prepared to handle it, and if so how?  At least I think\n> it deserves a comment here, or in rebase_parse_value(), to say (1)\n> that unknown rebase value is treated as false for most of the code\n> that do not need to differentiate between false and unknown, and (2)\n> that assigning a negative value to REBASE_INVALID and always\n> checking if the value is the same or greater than REBASE_TRUE helps\n> to maintain the convention.\n\nIts true that we never had 'info->rebase == REBASE_INVALID', but the\nprevious code also considered unknown values as false. 'info' is\nallocated with 'xcalloc', thus 'info->rebase' defaults to false. Thus\nit remains false.\n\nWhile my change may set 'info->rebase' implicitly to 'REBASE_INVALID'\nI also changed all truth value checks to '>= REBASE_TRUE'. Therefore,\n(and I must admit) incidentally, I did not introduced a function\nchange. Both versions handle unknown '.rebase' values as false.\n\nIf this is the expected behavior, I will add a comment to the line,\nwith that finding. If not, I will map 'REBASE_INVALID' to\n'REBASE_TRUE' in that case.\n\nBert\n"},{"id":"390229","messageId":"CAKPyHN0=jBc1PYC2jSp0SV7EuMwmRb_RRifmK66KTqVtP5oFRQ@mail.gmail.com","threadId":"52672","inReplyTo":"CAOjrSZsuPUc7kDPh6wTDMq10b2QM0R2Uq7-0TQ=W76yjk-eoJA@mail.gmail.com","subject":"Re: [PATCH 5/7] [RFC] config: make `scope_name` global as `config_scope_name`","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-22T07:37:21Z","receivedAt":"2020-01-22T07:37:35Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Wed, Jan 22, 2020 at 1:12 AM Matt Rogers <mattr94@gmail.com> wrote:\n>\n> Logos good to me...\n>\n> As I'm a bit new, what would be the best way for me to work this into\n> my workflow?\n\nif you have done that change already locally, then you can ignore my\npatch. I will wait for your re-roll and put my changes on top of\nyours. If not, you could replace your patch with this one in your\nseries. Your call.\n\nBert\n\n>\n> On Tue, Jan 21, 2020 at 4:25 AM Bert Wesarg <bert.wesarg@googlemail.com> wrote:\n> >\n> > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n> > ---\n> > Will be replaced by Matthew Rogers.\n> >\n> > Cc: Matthew Rogers <mattr94@gmail.com>\n> > ---\n> >  config.c               | 16 ++++++++++++++++\n> >  config.h               |  1 +\n> >  t/helper/test-config.c | 17 +----------------\n> >  3 files changed, 18 insertions(+), 16 deletions(-)\n> >\n> > diff --git a/config.c b/config.c\n> > index d75f88ca0c..4c461bb7a3 100644\n> > --- a/config.c\n> > +++ b/config.c\n> > @@ -3317,6 +3317,22 @@ enum config_scope current_config_scope(void)\n> >                 return current_parsing_scope;\n> >  }\n> >\n> > +const char *config_scope_name(enum config_scope scope)\n> > +{\n> > +       switch (scope) {\n> > +       case CONFIG_SCOPE_SYSTEM:\n> > +               return \"system\";\n> > +       case CONFIG_SCOPE_GLOBAL:\n> > +               return \"global\";\n> > +       case CONFIG_SCOPE_REPO:\n> > +               return \"repo\";\n> > +       case CONFIG_SCOPE_CMDLINE:\n> > +               return \"cmdline\";\n> > +       default:\n> > +               return \"unknown\";\n> > +       }\n> > +}\n> > +\n> >  int lookup_config(const char **mapping, int nr_mapping, const char *var)\n> >  {\n> >         int i;\n> > diff --git a/config.h b/config.h\n> > index 91fd4c5e96..c063f33ff6 100644\n> > --- a/config.h\n> > +++ b/config.h\n> > @@ -301,6 +301,7 @@ enum config_scope {\n> >         CONFIG_SCOPE_REPO,\n> >         CONFIG_SCOPE_CMDLINE,\n> >  };\n> > +const char *config_scope_name(enum config_scope scope);\n> >\n> >  enum config_scope current_config_scope(void);\n> >  const char *current_config_origin_type(void);\n> > diff --git a/t/helper/test-config.c b/t/helper/test-config.c\n> > index 214003d5b2..1e3bc7c8f4 100644\n> > --- a/t/helper/test-config.c\n> > +++ b/t/helper/test-config.c\n> > @@ -37,21 +37,6 @@\n> >   *\n> >   */\n> >\n> > -static const char *scope_name(enum config_scope scope)\n> > -{\n> > -       switch (scope) {\n> > -       case CONFIG_SCOPE_SYSTEM:\n> > -               return \"system\";\n> > -       case CONFIG_SCOPE_GLOBAL:\n> > -               return \"global\";\n> > -       case CONFIG_SCOPE_REPO:\n> > -               return \"repo\";\n> > -       case CONFIG_SCOPE_CMDLINE:\n> > -               return \"cmdline\";\n> > -       default:\n> > -               return \"unknown\";\n> > -       }\n> > -}\n> >  static int iterate_cb(const char *var, const char *value, void *data)\n> >  {\n> >         static int nr;\n> > @@ -63,7 +48,7 @@ static int iterate_cb(const char *var, const char *value, void *data)\n> >         printf(\"value=%s\\n\", value ? value : \"(null)\");\n> >         printf(\"origin=%s\\n\", current_config_origin_type());\n> >         printf(\"name=%s\\n\", current_config_name());\n> > -       printf(\"scope=%s\\n\", scope_name(current_config_scope()));\n> > +       printf(\"scope=%s\\n\", config_scope_name(current_config_scope()));\n> >\n> >         return 0;\n> >  }\n> > --\n> > 2.24.1.497.g9abd7b20b4.dirty\n> >\n>\n>\n> --\n> Matthew Rogers\n"},{"id":"390235","messageId":"CAKPyHN3=pw8qiL_jut9Say7qz54ceR0hNCNoJxbzJyR73qCBGg@mail.gmail.com","threadId":"52672","inReplyTo":"cover.1579598053.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH 0/7] remote rename: improve handling of configuration values","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-22T15:26:32Z","receivedAt":"2020-01-22T15:26:51Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"All,\n\nI think 'git remote remove X' needs similar improvements to\n'handle.*.pushremote = X' and 'push.default = X'. Will be handled in\nthe re-roll.\n\nBert\n\nOn Tue, Jan 21, 2020 at 10:24 AM Bert Wesarg <bert.wesarg@googlemail.com> wrote:\n>\n> While fixing that 'git remote rename X Y' does not rename the values for\n> 'branch.*.pushRemote', it opened the possibility to more improvements in\n> this area:\n>\n>  - 'remote rename' did not accept single-letter abbreviations for\n>    'branch.*.rebase' like 'pull --rebase' does\n>\n>  - minor clean-ups the config callback\n>\n>  - patch 5 will be replaced by/rebased on Matthew's work in 'config: allow user to\n>    know scope of config options', once 'config_scope_name' is available\n>\n>  - gently handling the rename of 'remote.pushDefault'\n>\n> Bert Wesarg (7):\n>   pull --rebase/remote rename: document and honor single-letter\n>     abbreviations rebase types\n>   remote: clean-up by returning early to avoid one indentation\n>   remote: clean-up config callback\n>   remote rename: rename branch.<name>.pushRemote config values too\n>   [RFC] config: make `scope_name` global as `config_scope_name`\n>   config: provide access to the current line number\n>   remote rename: gently handle remote.pushDefault config\n>\n>  Documentation/config/branch.txt |   7 +-\n>  Documentation/config/pull.txt   |   7 +-\n>  Makefile                        |   1 +\n>  builtin/pull.c                  |  29 +-----\n>  builtin/remote.c                | 168 +++++++++++++++++++++-----------\n>  config.c                        |  24 +++++\n>  config.h                        |   2 +\n>  rebase.c                        |  24 +++++\n>  rebase.h                        |  15 +++\n>  t/helper/test-config.c          |  18 +---\n>  t/t1308-config-set.sh           |  14 ++-\n>  t/t5505-remote.sh               |  52 +++++++++-\n>  12 files changed, 254 insertions(+), 107 deletions(-)\n>  create mode 100644 rebase.c\n>  create mode 100644 rebase.h\n>\n> --\n> 2.24.1.497.g9abd7b20b4.dirty\n>\n"},{"id":"390240","messageId":"xmqqh80n6zvp.fsf@gitster-ct.c.googlers.com","threadId":"52672","inReplyTo":"CAKPyHN0_AVOo_6bdHvy_J9ebnBpSD2NECBiLZ7g=4TcMvfZgYw@mail.gmail.com","subject":"Re: [PATCH 1/7] pull --rebase/remote rename: document and honor single-letter abbreviations rebase types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-22T19:43:22Z","receivedAt":"2020-01-22T19:43:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg <bert.wesarg@googlemail.com> writes:\n\n> Dear Junio,\n>\n> On Wed, Jan 22, 2020 at 12:26 AM Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Bert Wesarg <bert.wesarg@googlemail.com> writes:\n>>\n>> > When 46af44b07d (pull --rebase=<type>: allow single-letter abbreviations\n>> > for the type, 2018-08-04) landed in Git, it had the side effect that\n>> > not only 'pull --rebase=<type>' accepted the single-letter abbreviations\n>> > but also the 'pull.rebase' and 'branch.<name>.rebase' configurations.\n>> >\n>> > Secondly, 'git remote rename' did not honor these single-letter\n>> > abbreviations when reading the 'branch.*.rebase' configurations.\n>>\n>> Hmph, do you mean s/Secondly/However/ instead?\n>\n> thanks, that now reads smoothly.\n>\n>> > @@ -305,17 +304,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)\n>> >                               space = strchr(value, ' ');\n>> >                       }\n>> >                       string_list_append(&info->merge, xstrdup(value));\n>> > -             } else {\n>> > -                     int v = git_parse_maybe_bool(value);\n>> > -                     if (v >= 0)\n>> > -                             info->rebase = v;\n>> > -                     else if (!strcmp(value, \"preserve\"))\n>> > -                             info->rebase = NORMAL_REBASE;\n>> > -                     else if (!strcmp(value, \"merges\"))\n>> > -                             info->rebase = REBASE_MERGES;\n>> > -                     else if (!strcmp(value, \"interactive\"))\n>> > -                             info->rebase = INTERACTIVE_REBASE;\n>> > -             }\n>> > +             } else\n>> > +                     info->rebase = rebase_parse_value(value);\n>>\n>> Here, we never had info->rebase == REBASE_INVALID.  The field was\n>> left intact when the configuration file had a rebase type that is\n>> not known to this version of git.  Now it has become possible that\n>> info->rebase to be REBASE_INVALID.  Would the code after this part\n>> returns be prepared to handle it, and if so how?  At least I think\n>> it deserves a comment here, or in rebase_parse_value(), to say (1)\n>> that unknown rebase value is treated as false for most of the code\n>> that do not need to differentiate between false and unknown, and (2)\n>> that assigning a negative value to REBASE_INVALID and always\n>> checking if the value is the same or greater than REBASE_TRUE helps\n>> to maintain the convention.\n>\n> Its true that we never had 'info->rebase == REBASE_INVALID', but the\n> previous code also considered unknown values as false. 'info' is\n> allocated with 'xcalloc', thus 'info->rebase' defaults to false. Thus\n> it remains false.\n\nYes, that is why I was not opposed to the new code.  It was just\nthat it was not clear, without some comments I suggested in the\nlatter half of my paragraph you responded above, why it is correct\nto unconditionally assign to info->rebase and the code the control\nreaches after this part gets executed does not need any adjustment\nand simply \"works\".\n\nThinking about it again, I think the two points I thought need\nhighlighting in the above belong to the in-code comment for the new\nhelper rebase_parse_value().\n\n    *** in rebase.h ***\n    enum rebase_type {\n            REBASE_INVALID = -1,\n            REBASE_FALSE = 0,\n            REBASE_TRUE,\n            REBASE_PRESERVE,\n            REBASE_MERGES,\n            REBASE_INTERACTIVE\n    };\n\n    /*\n     * Parses textual value for pull.rebase, branch.<name>.rebase, etc.\n     * Unrecognised value yields REBASE_INVALID, which traditionally is\n     * treated the same way as REBASE_FALSE.\n     *\n     * The callers that care if (any) rebase is requested should say\n     *   if (REBASE_TRUE <= rebase_parse_value(string))\n     *\n     * The callers that want to differenciate an unrecognised value and\n     * false can do so by treating _INVALID and _FALSE differently.\n     */\n    enum rebase_type rebase_parse_value(const char *value);\n\nor something like that, perhaps.\n"},{"id":"390251","messageId":"CAOjrSZsn1KArNp0Dj-VtiEq9yVdxuc+MJHa_k0uo10cgn=PWOA@mail.gmail.com","threadId":"52672","inReplyTo":"CAKPyHN0=jBc1PYC2jSp0SV7EuMwmRb_RRifmK66KTqVtP5oFRQ@mail.gmail.com","subject":"Re: [PATCH 5/7] [RFC] config: make `scope_name` global as `config_scope_name`","fromName":"Matt Rogers","fromEmail":"mattr94@gmail.com","sentAt":"2020-01-23T01:30:29Z","receivedAt":"2020-01-23T01:30:42Z","isPatch":true,"sender":{"key":"mattr94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5719846?v=4"},"body":"I'll just put it into my local changes, and then reroll.  I should\nhave it up within a couple of days at the latest.\n"},{"id":"390315","messageId":"xmqqk15h3hfd.fsf@gitster-ct.c.googlers.com","threadId":"52672","inReplyTo":"59b97032fa158ccc9aee9d52b9cb969cd8df6a5f.1579598053.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH 2/7] remote: clean-up by returning early to avoid one indentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-23T23:02:30Z","receivedAt":"2020-01-23T23:02:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg <bert.wesarg@googlemail.com> writes:\n\n> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n>\n> ---\n> Cc: Junio C Hamano <gitster@pobox.com>\n> ---\n>  builtin/remote.c | 86 +++++++++++++++++++++++++-----------------------\n>  1 file changed, 44 insertions(+), 42 deletions(-)\n>\n> diff --git a/builtin/remote.c b/builtin/remote.c\n> index 2830c4ab33..a8bdaca4f4 100644\n> --- a/builtin/remote.c\n> +++ b/builtin/remote.c\n> @@ -263,50 +263,52 @@ static const char *abbrev_ref(const char *name, const char *prefix)\n>  \n>  static int config_read_branches(const char *key, const char *value, void *cb)\n>  {\n> -\tif (starts_with(key, \"branch.\")) {\n> -\t\tconst char *orig_key = key;\n> -\t\tchar *name;\n> -\t\tstruct string_list_item *item;\n> -\t\tstruct branch_info *info;\n> -\t\tenum { REMOTE, MERGE, REBASE } type;\n> -\t\tsize_t key_len;\n> -\n> -\t\tkey += 7;\n> -\t\tif (strip_suffix(key, \".remote\", &key_len)) {\n> -\t\t\tname = xmemdupz(key, key_len);\n> -\t\t\ttype = REMOTE;\n> -\t\t} else if (strip_suffix(key, \".merge\", &key_len)) {\n> -\t\t\tname = xmemdupz(key, key_len);\n> -\t\t\ttype = MERGE;\n> -\t\t} else if (strip_suffix(key, \".rebase\", &key_len)) {\n> -\t\t\tname = xmemdupz(key, key_len);\n> -\t\t\ttype = REBASE;\n> -\t\t} else\n> -\t\t\treturn 0;\n> +\tif (!starts_with(key, \"branch.\"))\n> +\t\treturn 0;\n\nThat's way too early.  We must have all decl/defn before the first\nstatement (see Documentation/CodingGuidelines).\n\n> -\t\titem = string_list_insert(&branch_list, name);\n> +\tconst char *orig_key = key;\n> +\tchar *name;\n> +\tstruct string_list_item *item;\n> +\tstruct branch_info *info;\n> +\tenum { REMOTE, MERGE, REBASE } type;\n> +\tsize_t key_len;\n\n"},{"id":"390316","messageId":"xmqqftg53hdm.fsf@gitster-ct.c.googlers.com","threadId":"52672","inReplyTo":"d10d3049ce9824f6925dddeb12cc130627a8c478.1579598053.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH 7/7] remote rename: gently handle remote.pushDefault config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-23T23:03:33Z","receivedAt":"2020-01-23T23:03:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg <bert.wesarg@googlemail.com> writes:\n\n> @@ -746,6 +769,26 @@ static int mv(int argc, const char **argv)\n>  \t\t\tdie(_(\"creating '%s' failed\"), buf.buf);\n>  \t}\n>  \tstring_list_clear(&remote_branches, 1);\n> +\n> +\tstruct push_default_info push_default;\n\nLikewise.  decl-after-stmt is not allowed.\n"},{"id":"390344","messageId":"CAKPyHN3F-c6Uy18PMkZ0YpwngN5HMMZtM3pma_Vj-WXVtvFuxw@mail.gmail.com","threadId":"52672","inReplyTo":"xmqqftg53hdm.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 7/7] remote rename: gently handle remote.pushDefault config","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-01-24T08:49:07Z","receivedAt":"2020-01-24T08:49:23Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Fri, Jan 24, 2020 at 12:03 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Bert Wesarg <bert.wesarg@googlemail.com> writes:\n>\n> > @@ -746,6 +769,26 @@ static int mv(int argc, const char **argv)\n> >                       die(_(\"creating '%s' failed\"), buf.buf);\n> >       }\n> >       string_list_clear(&remote_branches, 1);\n> > +\n> > +     struct push_default_info push_default;\n>\n> Likewise.  decl-after-stmt is not allowed.\n\nthanks. Its time for a re-roll.\n\nBert\n"}]}