{"thread":{"id":"47098","subject":"[RFC PATCH v3 0/4] give more useful error messages while renaming branch","startedAt":"2017-11-02T06:53:04Z","lastAt":"2017-11-02T06:55:16Z","messageCount":6,"participants":["Kaartic Sivaraam"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"331647","messageId":"20171102065242.25299-1-kaartic.sivaraam@gmail.com","threadId":"47098","inReplyTo":null,"subject":"[RFC PATCH v3 0/4] give more useful error messages while renaming branch","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-11-02T06:52:38Z","receivedAt":"2017-11-02T06:53:04Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"In builtin/branch, the error messages weren't handled directly by the branch\nrenaming function and was left to the other function. Though this avoids\nredundancy this gave unclear error messages in some cases. So, make builtin/branch\ngive more useful error messages.\n\nChanges in v3:\n\n\tIncorporated suggestions from v2 to improve code and commit message. To\n\tbe more precise about the code part,\n\n\tIn 2/4 slightly re-ordered the parameters to move the flag parameters to\n\tthe end.\n\n\tIn 3/4, changed the return type of the branchname validation functions to\n\tbe the enum (whose values they return) as suggested by Stefan.\n\n\tDropped the PATCH 3/5 of v2 as there was another series[1] that did the\n\trefactor and got merged to 'next'. I have now re-rolled the series over\n\t'next' [pointing at 273055501 (Sync with master, 2017-10-24)].\n \n\tThis has made the code in 3/4 a little clumsy (at least to me) as I\n\ttried to achieve to achieve what the previous patches did with the new\n\tvalidate*_branchname functionS. Let me know, if it looks too bad.\n\nSo this could go on top of 'next' without any conflicts but in case I\nmissed something, let me know. The series could be found in my fork[2].\n\n\nAny feedback welcome.\n\nThanks,\nKaartic\n\n[1] : https://public-inbox.org/git/20171013051132.3973-1-gitster@pobox.com\n\n[2] : https://github.com/sivaraam/git/tree/work/branch-revamp\n\n\nKaartic Sivaraam (4):\n  branch: improve documentation and naming of 'create_branch()'\n  branch: re-order function arguments to group related arguments\n  branch: introduce dont_fail parameter for branchname validation\n  builtin/branch: give more useful error messages when renaming\n\n branch.c           | 63 ++++++++++++++++++++++++++++++------------------------\n branch.h           | 57 ++++++++++++++++++++++++++++++++++++++----------\n builtin/branch.c   | 49 ++++++++++++++++++++++++++++++++++--------\n builtin/checkout.c | 11 +++++-----\n 4 files changed, 127 insertions(+), 53 deletions(-)\n\n-- \n2.15.0.rc2.401.g3db9995f9\n"},{"id":"331648","messageId":"20171102065242.25299-2-kaartic.sivaraam@gmail.com","threadId":"47098","inReplyTo":"20171102065242.25299-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH v3 1/4] branch: improve documentation and naming of 'create_branch()'","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-11-02T06:52:39Z","receivedAt":"2017-11-02T06:53:09Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n\nThe documentation for 'create_branch()' was incomplete as it didn't say\nwhat certain parameters were used for. Further a parameter name wasn't\nvery communicative.\n\nSo, add missing documentation for the sake of completeness and easy\nreference. Also, rename the concerned parameter to make its name more\ncommunicative.\n\nSigned-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n---\n branch.c | 4 ++--\n branch.h | 7 ++++++-\n 2 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex fe1e1c367..ea6e2b359 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -244,7 +244,7 @@ N_(\"\\n\"\n \"\\\"git push -u\\\" to set the upstream config as you push.\");\n \n void create_branch(const char *name, const char *start_name,\n-\t\t   int force, int reflog, int clobber_head,\n+\t\t   int force, int reflog, int clobber_head_ok,\n \t\t   int quiet, enum branch_track track)\n {\n \tstruct commit *commit;\n@@ -258,7 +258,7 @@ void create_branch(const char *name, const char *start_name,\n \tif (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)\n \t\texplicit_tracking = 1;\n \n-\tif ((track == BRANCH_TRACK_OVERRIDE || clobber_head)\n+\tif ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok)\n \t    ? validate_branchname(name, &ref)\n \t    : validate_new_branchname(name, &ref, force)) {\n \t\tif (!force)\ndiff --git a/branch.h b/branch.h\nindex be5e5d130..1512b78d1 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -15,12 +15,17 @@\n  *\n  *   - reflog creates a reflog for the branch\n  *\n+ *   - clobber_head_ok allows the currently checked out (hence existing)\n+ *     branch to be overwritten; without 'force', it has no effect.\n+ *\n+ *   - quiet suppresses tracking information\n+ *\n  *   - track causes the new branch to be configured to merge the remote branch\n  *     that start_name is a tracking branch for (if any).\n  */\n void create_branch(const char *name, const char *start_name,\n \t\t   int force, int reflog,\n-\t\t   int clobber_head, int quiet, enum branch_track track);\n+\t\t   int clobber_head_ok, int quiet, enum branch_track track);\n \n /*\n  * Check if 'name' can be a valid name for a branch; die otherwise.\n-- \n2.15.0.461.gf957c703b.dirty\n\n"},{"id":"331649","messageId":"20171102065242.25299-3-kaartic.sivaraam@gmail.com","threadId":"47098","inReplyTo":"20171102065242.25299-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH v3 2/4] branch: re-order function arguments to group related arguments","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-11-02T06:52:40Z","receivedAt":"2017-11-02T06:53:12Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n\nThe ad-hoc patches to add new arguments to a function when needed\nresulted in the related arguments not being close to each other.\nThis misleads the person reading the code to believe that there isn't\nmuch relation between those arguments while it's not the case.\n\nSo, re-order the arguments to keep the related arguments close to each\nother.\n\nSigned-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n---\n branch.c           |  4 ++--\n branch.h           | 14 +++++++-------\n builtin/branch.c   |  4 ++--\n builtin/checkout.c |  6 +++---\n 4 files changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex ea6e2b359..7c8093041 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -244,8 +244,8 @@ N_(\"\\n\"\n \"\\\"git push -u\\\" to set the upstream config as you push.\");\n \n void create_branch(const char *name, const char *start_name,\n-\t\t   int force, int reflog, int clobber_head_ok,\n-\t\t   int quiet, enum branch_track track)\n+\t\t   enum branch_track track, int force, int clobber_head_ok,\n+\t\t   int reflog, int quiet)\n {\n \tstruct commit *commit;\n \tstruct object_id oid;\ndiff --git a/branch.h b/branch.h\nindex 1512b78d1..85052628b 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -11,21 +11,21 @@\n  *   - start_name is the name of the existing branch that the new branch should\n  *     start from\n  *\n- *   - force enables overwriting an existing (non-head) branch\n+ *   - track causes the new branch to be configured to merge the remote branch\n+ *     that start_name is a tracking branch for (if any).\n  *\n- *   - reflog creates a reflog for the branch\n+ *   - force enables overwriting an existing (non-head) branch\n  *\n  *   - clobber_head_ok allows the currently checked out (hence existing)\n  *     branch to be overwritten; without 'force', it has no effect.\n  *\n- *   - quiet suppresses tracking information\n+ *   - reflog creates a reflog for the branch\n  *\n- *   - track causes the new branch to be configured to merge the remote branch\n- *     that start_name is a tracking branch for (if any).\n+ *   - quiet suppresses tracking information\n  */\n void create_branch(const char *name, const char *start_name,\n-\t\t   int force, int reflog,\n-\t\t   int clobber_head_ok, int quiet, enum branch_track track);\n+\t\t   enum branch_track track, int force, int clobber_head_ok,\n+\t\t   int reflog, int quiet);\n \n /*\n  * Check if 'name' can be a valid name for a branch; die otherwise.\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 5be40b384..df06ac968 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -766,7 +766,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\t * create_branch takes care of setting up the tracking\n \t\t * info and making sure new_upstream is correct\n \t\t */\n-\t\tcreate_branch(branch->name, new_upstream, 0, 0, 0, quiet, BRANCH_TRACK_OVERRIDE);\n+\t\tcreate_branch(branch->name, new_upstream, BRANCH_TRACK_OVERRIDE, 0, 0, 0, quiet);\n \t} else if (unset_upstream) {\n \t\tstruct branch *branch = branch_get(argv[0]);\n \t\tstruct strbuf buf = STRBUF_INIT;\n@@ -806,7 +806,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead.\"));\n \n \t\tcreate_branch(argv[0], (argc == 2) ? argv[1] : head,\n-\t\t\t      force, reflog, 0, quiet, track);\n+\t\t\t      track, force, 0, reflog, quiet);\n \n \t} else\n \t\tusage_with_options(builtin_branch_usage, options);\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 8546d630b..5c34a9a0d 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -639,11 +639,11 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \t\t}\n \t\telse\n \t\t\tcreate_branch(opts->new_branch, new->name,\n+\t\t\t\t      opts->track,\n+\t\t\t\t      opts->new_branch_force ? 1 : 0,\n \t\t\t\t      opts->new_branch_force ? 1 : 0,\n \t\t\t\t      opts->new_branch_log,\n-\t\t\t\t      opts->new_branch_force ? 1 : 0,\n-\t\t\t\t      opts->quiet,\n-\t\t\t\t      opts->track);\n+\t\t\t\t      opts->quiet);\n \t\tnew->name = opts->new_branch;\n \t\tsetup_branch_path(new);\n \t}\n-- \n2.15.0.461.gf957c703b.dirty\n\n"},{"id":"331650","messageId":"20171102065242.25299-4-kaartic.sivaraam@gmail.com","threadId":"47098","inReplyTo":"20171102065242.25299-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH v3 3/4] branch: introduce dont_fail parameter for branchname validation","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-11-02T06:52:41Z","receivedAt":"2017-11-02T06:53:17Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"This parameter allows the branchname validation functions to\noptionally return a flag specifying the reason for failure, when\nrequested. This allows the caller to know why it was about to die.\nThis allows more useful error messages to be given to the user when\ntrying to rename a branch.\n\nThe flags are specified in the form of an enum and values for success\nflags have been assigned explicitly to clearly express that certain\ncallers rely on those values and they cannot be arbitrary.\n\nOnly the logic has been added but no caller has been made to use it, yet.\nSo, no functional changes.\n\nSigned-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\nSigned-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n---\n branch.c           | 62 +++++++++++++++++++++++++++++++-----------------------\n branch.h           | 60 ++++++++++++++++++++++++++++++++++++++++++----------\n builtin/branch.c   |  4 ++--\n builtin/checkout.c |  5 +++--\n 4 files changed, 90 insertions(+), 41 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 7c8093041..7db2e3296 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -178,41 +178,51 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)\n \treturn 0;\n }\n \n-/*\n- * Check if 'name' can be a valid name for a branch; die otherwise.\n- * Return 1 if the named branch already exists; return 0 otherwise.\n- * Fill ref with the full refname for the branch.\n- */\n-int validate_branchname(const char *name, struct strbuf *ref)\n+enum branch_validation_result validate_branchname(const char *name, struct strbuf *ref, unsigned dont_fail)\n {\n-\tif (strbuf_check_branch_ref(ref, name))\n-\t\tdie(_(\"'%s' is not a valid branch name.\"), name);\n+\tif (strbuf_check_branch_ref(ref, name)) {\n+\t\tif (dont_fail)\n+\t\t\treturn INVALID_BRANCH_NAME;\n+\t\telse\n+\t\t\tdie(_(\"'%s' is not a valid branch name.\"), name);\n+\t}\n \n-\treturn ref_exists(ref->buf);\n+\tif(ref_exists(ref->buf))\n+\t\treturn BRANCH_EXISTS;\n+\telse\n+\t\treturn BRANCH_DOESNT_EXIST;\n }\n \n-/*\n- * Check if a branch 'name' can be created as a new branch; die otherwise.\n- * 'force' can be used when it is OK for the named branch already exists.\n- * Return 1 if the named branch already exists; return 0 otherwise.\n- * Fill ref with the full refname for the branch.\n- */\n-int validate_new_branchname(const char *name, struct strbuf *ref, int force)\n+enum branch_validation_result validate_new_branchname(const char *name, struct strbuf *ref, int force, unsigned dont_fail)\n {\n \tconst char *head;\n \n-\tif (!validate_branchname(name, ref))\n-\t\treturn 0;\n+\tif(dont_fail) {\n+\t\tenum branch_validation_result res = validate_branchname(name, ref, 1);\n+\t\tif (res == INVALID_BRANCH_NAME || res == BRANCH_DOESNT_EXIST)\n+\t\t\t\treturn res;\n+\t} else {\n+\t\tif(validate_branchname(name, ref, 0) == BRANCH_DOESNT_EXIST)\n+\t\t\treturn BRANCH_DOESNT_EXIST;\n+\t}\n \n-\tif (!force)\n-\t\tdie(_(\"A branch named '%s' already exists.\"),\n-\t\t    ref->buf + strlen(\"refs/heads/\"));\n+\tif (!force) {\n+\t\tif (dont_fail)\n+\t\t\treturn BRANCH_EXISTS_NO_FORCE;\n+\t\telse\n+\t\t\tdie(_(\"A branch named '%s' already exists.\"),\n+\t\t\t\tref->buf + strlen(\"refs/heads/\"));\n+\t}\n \n \thead = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n-\tif (!is_bare_repository() && head && !strcmp(head, ref->buf))\n-\t\tdie(_(\"Cannot force update the current branch.\"));\n+\tif (!is_bare_repository() && head && !strcmp(head, ref->buf)) {\n+\t\tif (dont_fail)\n+\t\t\treturn CANNOT_FORCE_UPDATE_CURRENT_BRANCH;\n+\t\telse\n+\t\t\tdie(_(\"Cannot force update the current branch.\"));\n+\t}\n \n-\treturn 1;\n+\treturn BRANCH_EXISTS;\n }\n \n static int check_tracking_branch(struct remote *remote, void *cb_data)\n@@ -259,8 +269,8 @@ void create_branch(const char *name, const char *start_name,\n \t\texplicit_tracking = 1;\n \n \tif ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok)\n-\t    ? validate_branchname(name, &ref)\n-\t    : validate_new_branchname(name, &ref, force)) {\n+\t    ? validate_branchname(name, &ref, 0)\n+\t    : validate_new_branchname(name, &ref, force, 0)) {\n \t\tif (!force)\n \t\t\tdont_change_ref = 1;\n \t\telse\ndiff --git a/branch.h b/branch.h\nindex 85052628b..0c178ec5a 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -27,20 +27,58 @@ void create_branch(const char *name, const char *start_name,\n \t\t   enum branch_track track, int force, int clobber_head_ok,\n \t\t   int reflog, int quiet);\n \n-/*\n- * Check if 'name' can be a valid name for a branch; die otherwise.\n- * Return 1 if the named branch already exists; return 0 otherwise.\n- * Fill ref with the full refname for the branch.\n- */\n-extern int validate_branchname(const char *name, struct strbuf *ref);\n+enum branch_validation_result {\n+\t/* Flags that convey that validation FAILED */\n+\tBRANCH_EXISTS_NO_FORCE = -3,\n+\tCANNOT_FORCE_UPDATE_CURRENT_BRANCH,\n+\tINVALID_BRANCH_NAME,\n+\t/* Flags that convey that validation SUCCEEDED */\n+\tBRANCH_DOESNT_EXIST = 0,\n+\tBRANCH_EXISTS = 1,\n+};\n \n /*\n- * Check if a branch 'name' can be created as a new branch; die otherwise.\n- * 'force' can be used when it is OK for the named branch already exists.\n- * Return 1 if the named branch already exists; return 0 otherwise.\n- * Fill ref with the full refname for the branch.\n+ * Check if 'name' can be a valid name for a branch; die otherwise.\n+ *\n+ *   - name is the new branch name\n+ *\n+ *   - ref is used to return the full refname for the branch\n+ *\n+ * The return values have the following meaning,\n+ *\n+ *   - If dont_fail is 0, the function dies in case of failure and returns flags of\n+ *     'branch_validation_result' that indicate status of the given branch. The positive\n+ *     non-zero flag implies that the branch exists.\n+ *\n+ *   - If dont_fail is 1, the function doesn't die in case of failure but returns flags\n+ *     of 'branch_validaton_result' that specify the reason for failure. The behaviour in case of\n+ *     success is same as above.\n+ *\n  */\n-extern int validate_new_branchname(const char *name, struct strbuf *ref, int force);\n+extern enum branch_validation_result validate_branchname(const char *name, struct strbuf *ref, unsigned dont_fail);\n+\n+/*\n+ * Check if a branch 'name' can be created as a new branch.\n+ *\n+ *   - name is the new branch name\n+ *\n+ *   - ref is used to return the full refname for the branch\n+ *\n+ *   - force can be used when it is OK if the named branch already exists.\n+ *     the currently checkout branch; with 'shouldnt_exist', it has no effect.\n+ *\n+ * The return values have the following meaning,\n+ *\n+ *   - If dont_fail is 0, the function dies in case of failure and returns flags of\n+ *     'branch_validation_result' that indicate that convey status of given branch. The positive\n+ *     non-zero flag implies that the branch can be force updated.\n+ *\n+ *   - If dont_fail is 1, the function doesn't die in case of failure but returns flags\n+ *     of 'branch_validaton_result' that specify the reason for failure. The behaviour in case of\n+ *     success is same as above.\n+ *\n+ */\n+extern enum branch_validation_result validate_new_branchname(const char *name, struct strbuf *ref, int force, unsigned dont_fail);\n \n /*\n  * Remove information about the state of working on the current\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex df06ac968..7018e5d75 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -487,9 +487,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \t * cause the worktree to become inconsistent with HEAD, so allow it.\n \t */\n \tif (!strcmp(oldname, newname))\n-\t\tvalidate_branchname(newname, &newref);\n+\t\tvalidate_branchname(newname, &newref, 0);\n \telse\n-\t\tvalidate_new_branchname(newname, &newref, force);\n+\t\tvalidate_new_branchname(newname, &newref, force, 0);\n \n \treject_rebase_or_bisect_branch(oldref.buf);\n \ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 5c34a9a0d..4adab3814 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1288,10 +1288,11 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tstruct strbuf buf = STRBUF_INIT;\n \n \t\tif (opts.new_branch_force)\n-\t\t\topts.branch_exists = validate_branchname(opts.new_branch, &buf);\n+\t\t\topts.branch_exists = validate_branchname(opts.new_branch, &buf, 0);\n \t\telse\n \t\t\topts.branch_exists =\n-\t\t\t\tvalidate_new_branchname(opts.new_branch, &buf, 0);\n+\t\t\t\tvalidate_new_branchname(opts.new_branch, &buf, 0, 0);\n+\n \t\tstrbuf_release(&buf);\n \t}\n \n-- \n2.15.0.461.gf957c703b.dirty\n\n"},{"id":"331651","messageId":"20171102065242.25299-5-kaartic.sivaraam@gmail.com","threadId":"47098","inReplyTo":"20171102065242.25299-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH v3 4/4] builtin/branch: give more useful error messages when renaming","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-11-02T06:52:42Z","receivedAt":"2017-11-02T06:53:19Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n\nWhen trying to rename an inexistent branch to with a name of a branch\nthat already exists the rename failed specifying the new branch name\nexists rather than specifying that the branch trying to be renamed\ndoesn't exist.\n\n    $ git branch -m tset master\n    fatal: A branch named 'master' already exists.\n\nIt's conventional to report that 'tset' doesn't exist rather than\nreporting that 'master' exists, the same way the 'mv' command does.\n\n    (hypothetical)\n    $ git branch -m tset master\n    fatal: branch 'tset' doesn't exist.\n\nThat has the problem that the error about an existing branch is shown\nonly after the user corrects the error about inexistent branch.\n\n    $ git branch -m test master\n    fatal: A branch named 'master' already exists.\n\nThis isn't useful either because the user would have corrected this error in\na single go if he had been told this alongside the first error. So, give\nmore useful error messages by giving errors about old branch name and new\nbranch name at the same time. This is possible as the branch name validation\nfunctions now return the reason they were about to die, when requested.\n\n    $ git branch -m tset master\n    fatal: branch 'tset' doesn't exist, and branch 'master' already exists\n\nNote: Thanks to the strbuf API that made it possible to easily construct\nthe composite error message strings!\n\nSigned-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n---\n builtin/branch.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 42 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 7018e5d75..c2bbf8c3d 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -458,11 +458,42 @@ static void reject_rebase_or_bisect_branch(const char *target)\n \tfree_worktrees(worktrees);\n }\n \n+static void get_error_msg(struct strbuf* error_msg, const char* oldname, unsigned old_branch_exists,\n+\t\t\t  const char* newname, enum branch_validation_result res)\n+{\n+\tconst char* connector_string = _(\", and \");\n+\n+\tif (!old_branch_exists) {\n+\t\tstrbuf_addf(error_msg, _(\"branch '%s' doesn't exist\"), oldname);\n+\t}\n+\n+\tswitch (res) {\n+\t\tcase BRANCH_EXISTS_NO_FORCE:\n+\t\t\tstrbuf_addf(error_msg, \"%s\", (!old_branch_exists) ? connector_string : \"\");\n+\t\t\tstrbuf_addf(error_msg,_(\"branch '%s' already exists\"), newname);\n+\t\t\tbreak;\n+\t\tcase CANNOT_FORCE_UPDATE_CURRENT_BRANCH:\n+\t\t\tstrbuf_addf(error_msg, \"%s\", (!old_branch_exists) ? connector_string : \"\");\n+\t\t\tstrbuf_addstr(error_msg, _(\"cannot force update the current branch\"));\n+\t\t\tbreak;\n+\t\tcase INVALID_BRANCH_NAME:\n+\t\t\tstrbuf_addf(error_msg, \"%s\", (!old_branch_exists) ? connector_string : \"\");\n+\t\t\tstrbuf_addf(error_msg, _(\"branch name '%s' is invalid\"), newname);\n+\t\t\tbreak;\n+\t\t/* not necessary to handle success cases */\n+\t\tcase BRANCH_EXISTS:\n+\t\tcase BRANCH_DOESNT_EXIST:\n+\t\t\tbreak;\n+\t}\n+}\n+\n static void copy_or_rename_branch(const char *oldname, const char *newname, int copy, int force)\n {\n \tstruct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;\n \tstruct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;\n \tint recovery = 0;\n+\tstruct strbuf error_msg = STRBUF_INIT, empty = STRBUF_INIT;\n+\tenum branch_validation_result res;\n \n \tif (!oldname) {\n \t\tif (copy)\n@@ -471,15 +502,13 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \t\t\tdie(_(\"cannot rename the current branch while not on any.\"));\n \t}\n \n-\tif (strbuf_check_branch_ref(&oldref, oldname)) {\n+\tif (strbuf_check_branch_ref(&oldref, oldname) && ref_exists(oldref.buf))\n+\t{\n \t\t/*\n \t\t * Bad name --- this could be an attempt to rename a\n \t\t * ref that we used to allow to be created by accident.\n \t\t */\n-\t\tif (ref_exists(oldref.buf))\n-\t\t\trecovery = 1;\n-\t\telse\n-\t\t\tdie(_(\"Invalid branch name: '%s'\"), oldname);\n+\t\trecovery = 1;\n \t}\n \n \t/*\n@@ -487,9 +516,13 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \t * cause the worktree to become inconsistent with HEAD, so allow it.\n \t */\n \tif (!strcmp(oldname, newname))\n-\t\tvalidate_branchname(newname, &newref, 0);\n+\t\tres = validate_branchname(newname, &newref, 1);\n \telse\n-\t\tvalidate_new_branchname(newname, &newref, force, 0);\n+\t\tres = validate_new_branchname(newname, &newref, force, 1);\n+\n+\tget_error_msg(&error_msg, oldname, ref_exists(oldref.buf), newname, res);\n+\tif (strbuf_cmp(&error_msg, &empty))\n+\t\tdie(\"%s\", error_msg.buf);\n \n \treject_rebase_or_bisect_branch(oldref.buf);\n \n@@ -530,6 +563,8 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \t\tdie(_(\"Branch is copied, but update of config-file failed\"));\n \tstrbuf_release(&oldsection);\n \tstrbuf_release(&newsection);\n+\tstrbuf_release(&error_msg);\n+\tstrbuf_release(&empty);\n }\n \n static GIT_PATH_FUNC(edit_description, \"EDIT_DESCRIPTION\")\n-- \n2.15.0.461.gf957c703b.dirty\n\n"},{"id":"331657","messageId":"1509605703.9254.13.camel@gmail.com","threadId":"47098","inReplyTo":"20171102065242.25299-1-kaartic.sivaraam@gmail.com","subject":"Re: [RFC PATCH v3 0/4] give more useful error messages while renaming branch","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-11-02T06:55:03Z","receivedAt":"2017-11-02T06:55:16Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"Sorry, ignore this mails. I actually have re-sent it to the correct\nthread.\n \n-- \nKaartic\n"}]}