[RFC PATCH v3 0/4] give more useful error messages while renaming branch

[RFC PATCH v3 0/4] give more useful error messages while renaming branch

[Kaartic Sivaraam]

In builtin/branch, the error messages weren't handled directly by the branch
renaming function and was left to the other function. Though this avoids
redundancy this gave unclear error messages in some cases. So, make builtin/branch
give more useful error messages.

Changes in v3:

	Incorporated suggestions from v2 to improve code and commit message. To
	be more precise about the code part,

	In 2/4 slightly re-ordered the parameters to move the flag parameters to
	the end.

	In 3/4, changed the return type of the branchname validation functions to
	be the enum (whose values they return) as suggested by Stefan.

	Dropped the PATCH 3/5 of v2 as there was another series[1] that did the
	refactor and got merged to 'next'. I have now re-rolled the series over
	'next' [pointing at 273055501 (Sync with master, 2017-10-24)].
 
	This has made the code in 3/4 a little clumsy (at least to me) as I
	tried to achieve to achieve what the previous patches did with the new
	validate*_branchname functionS. Let me know, if it looks too bad.

So this could go on top of 'next' without any conflicts but in case I
missed something, let me know. The series could be found in my fork[2].


Any feedback welcome.

Thanks,
Kaartic

[1] : https://public-inbox.org/git/20171013051132.3973-1-gitster@pobox.com

[2] : https://github.com/sivaraam/git/tree/work/branch-revamp


Kaartic Sivaraam (4):
  branch: improve documentation and naming of 'create_branch()'
  branch: re-order function arguments to group related arguments
  branch: introduce dont_fail parameter for branchname validation
  builtin/branch: give more useful error messages when renaming

 branch.c           | 63 ++++++++++++++++++++++++++++++------------------------
 branch.h           | 57 ++++++++++++++++++++++++++++++++++++++----------
 builtin/branch.c   | 49 ++++++++++++++++++++++++++++++++++--------
 builtin/checkout.c | 11 +++++-----
 4 files changed, 127 insertions(+), 53 deletions(-)

-- 
2.15.0.rc2.401.g3db9995f9

[RFC PATCH v3 1/4] branch: improve documentation and naming of 'create_branch()'

[Kaartic Sivaraam]

From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>

The documentation for 'create_branch()' was incomplete as it didn't say
what certain parameters were used for. Further a parameter name wasn't
very communicative.

So, add missing documentation for the sake of completeness and easy
reference. Also, rename the concerned parameter to make its name more
communicative.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 branch.c | 4 ++--
 branch.h | 7 ++++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/branch.c b/branch.c
index fe1e1c367..ea6e2b359 100644
--- a/branch.c
+++ b/branch.c
@@ -244,7 +244,7 @@ N_("\n"
 "\"git push -u\" to set the upstream config as you push.");
 
 void create_branch(const char *name, const char *start_name,
-		   int force, int reflog, int clobber_head,
+		   int force, int reflog, int clobber_head_ok,
 		   int quiet, enum branch_track track)
 {
 	struct commit *commit;
@@ -258,7 +258,7 @@ void create_branch(const char *name, const char *start_name,
 	if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)
 		explicit_tracking = 1;
 
-	if ((track == BRANCH_TRACK_OVERRIDE || clobber_head)
+	if ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok)
 	    ? validate_branchname(name, &ref)
 	    : validate_new_branchname(name, &ref, force)) {
 		if (!force)
diff --git a/branch.h b/branch.h
index be5e5d130..1512b78d1 100644
--- a/branch.h
+++ b/branch.h
@@ -15,12 +15,17 @@
  *
  *   - reflog creates a reflog for the branch
  *
+ *   - clobber_head_ok allows the currently checked out (hence existing)
+ *     branch to be overwritten; without 'force', it has no effect.
+ *
+ *   - quiet suppresses tracking information
+ *
  *   - track causes the new branch to be configured to merge the remote branch
  *     that start_name is a tracking branch for (if any).
  */
 void create_branch(const char *name, const char *start_name,
 		   int force, int reflog,
-		   int clobber_head, int quiet, enum branch_track track);
+		   int clobber_head_ok, int quiet, enum branch_track track);
 
 /*
  * Check if 'name' can be a valid name for a branch; die otherwise.
-- 
2.15.0.461.gf957c703b.dirty

[RFC PATCH v3 2/4] branch: re-order function arguments to group related arguments

[Kaartic Sivaraam]

From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>

The ad-hoc patches to add new arguments to a function when needed
resulted in the related arguments not being close to each other.
This misleads the person reading the code to believe that there isn't
much relation between those arguments while it's not the case.

So, re-order the arguments to keep the related arguments close to each
other.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 branch.c           |  4 ++--
 branch.h           | 14 +++++++-------
 builtin/branch.c   |  4 ++--
 builtin/checkout.c |  6 +++---
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/branch.c b/branch.c
index ea6e2b359..7c8093041 100644
--- a/branch.c
+++ b/branch.c
@@ -244,8 +244,8 @@ N_("\n"
 "\"git push -u\" to set the upstream config as you push.");
 
 void create_branch(const char *name, const char *start_name,
-		   int force, int reflog, int clobber_head_ok,
-		   int quiet, enum branch_track track)
+		   enum branch_track track, int force, int clobber_head_ok,
+		   int reflog, int quiet)
 {
 	struct commit *commit;
 	struct object_id oid;
diff --git a/branch.h b/branch.h
index 1512b78d1..85052628b 100644
--- a/branch.h
+++ b/branch.h
@@ -11,21 +11,21 @@
  *   - start_name is the name of the existing branch that the new branch should
  *     start from
  *
- *   - force enables overwriting an existing (non-head) branch
+ *   - track causes the new branch to be configured to merge the remote branch
+ *     that start_name is a tracking branch for (if any).
  *
- *   - reflog creates a reflog for the branch
+ *   - force enables overwriting an existing (non-head) branch
  *
  *   - clobber_head_ok allows the currently checked out (hence existing)
  *     branch to be overwritten; without 'force', it has no effect.
  *
- *   - quiet suppresses tracking information
+ *   - reflog creates a reflog for the branch
  *
- *   - track causes the new branch to be configured to merge the remote branch
- *     that start_name is a tracking branch for (if any).
+ *   - quiet suppresses tracking information
  */
 void create_branch(const char *name, const char *start_name,
-		   int force, int reflog,
-		   int clobber_head_ok, int quiet, enum branch_track track);
+		   enum branch_track track, int force, int clobber_head_ok,
+		   int reflog, int quiet);
 
 /*
  * Check if 'name' can be a valid name for a branch; die otherwise.
diff --git a/builtin/branch.c b/builtin/branch.c
index 5be40b384..df06ac968 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -766,7 +766,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		 * create_branch takes care of setting up the tracking
 		 * info and making sure new_upstream is correct
 		 */
-		create_branch(branch->name, new_upstream, 0, 0, 0, quiet, BRANCH_TRACK_OVERRIDE);
+		create_branch(branch->name, new_upstream, BRANCH_TRACK_OVERRIDE, 0, 0, 0, quiet);
 	} else if (unset_upstream) {
 		struct branch *branch = branch_get(argv[0]);
 		struct strbuf buf = STRBUF_INIT;
@@ -806,7 +806,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
 
 		create_branch(argv[0], (argc == 2) ? argv[1] : head,
-			      force, reflog, 0, quiet, track);
+			      track, force, 0, reflog, quiet);
 
 	} else
 		usage_with_options(builtin_branch_usage, options);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8546d630b..5c34a9a0d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -639,11 +639,11 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
 		}
 		else
 			create_branch(opts->new_branch, new->name,
+				      opts->track,
+				      opts->new_branch_force ? 1 : 0,
 				      opts->new_branch_force ? 1 : 0,
 				      opts->new_branch_log,
-				      opts->new_branch_force ? 1 : 0,
-				      opts->quiet,
-				      opts->track);
+				      opts->quiet);
 		new->name = opts->new_branch;
 		setup_branch_path(new);
 	}
-- 
2.15.0.461.gf957c703b.dirty

[RFC PATCH v3 3/4] branch: introduce dont_fail parameter for branchname validation

[Kaartic Sivaraam]

This parameter allows the branchname validation functions to
optionally return a flag specifying the reason for failure, when
requested. This allows the caller to know why it was about to die.
This allows more useful error messages to be given to the user when
trying to rename a branch.

The flags are specified in the form of an enum and values for success
flags have been assigned explicitly to clearly express that certain
callers rely on those values and they cannot be arbitrary.

Only the logic has been added but no caller has been made to use it, yet.
So, no functional changes.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---
 branch.c           | 62 +++++++++++++++++++++++++++++++-----------------------
 branch.h           | 60 ++++++++++++++++++++++++++++++++++++++++++----------
 builtin/branch.c   |  4 ++--
 builtin/checkout.c |  5 +++--
 4 files changed, 90 insertions(+), 41 deletions(-)

diff --git a/branch.c b/branch.c
index 7c8093041..7db2e3296 100644
--- a/branch.c
+++ b/branch.c
@@ -178,41 +178,51 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)
 	return 0;
 }
 
-/*
- * Check if 'name' can be a valid name for a branch; die otherwise.
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
-int validate_branchname(const char *name, struct strbuf *ref)
+enum branch_validation_result validate_branchname(const char *name, struct strbuf *ref, unsigned dont_fail)
 {
-	if (strbuf_check_branch_ref(ref, name))
-		die(_("'%s' is not a valid branch name."), name);
+	if (strbuf_check_branch_ref(ref, name)) {
+		if (dont_fail)
+			return INVALID_BRANCH_NAME;
+		else
+			die(_("'%s' is not a valid branch name."), name);
+	}
 
-	return ref_exists(ref->buf);
+	if(ref_exists(ref->buf))
+		return BRANCH_EXISTS;
+	else
+		return BRANCH_DOESNT_EXIST;
 }
 
-/*
- * Check if a branch 'name' can be created as a new branch; die otherwise.
- * 'force' can be used when it is OK for the named branch already exists.
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
-int validate_new_branchname(const char *name, struct strbuf *ref, int force)
+enum branch_validation_result validate_new_branchname(const char *name, struct strbuf *ref, int force, unsigned dont_fail)
 {
 	const char *head;
 
-	if (!validate_branchname(name, ref))
-		return 0;
+	if(dont_fail) {
+		enum branch_validation_result res = validate_branchname(name, ref, 1);
+		if (res == INVALID_BRANCH_NAME || res == BRANCH_DOESNT_EXIST)
+				return res;
+	} else {
+		if(validate_branchname(name, ref, 0) == BRANCH_DOESNT_EXIST)
+			return BRANCH_DOESNT_EXIST;
+	}
 
-	if (!force)
-		die(_("A branch named '%s' already exists."),
-		    ref->buf + strlen("refs/heads/"));
+	if (!force) {
+		if (dont_fail)
+			return BRANCH_EXISTS_NO_FORCE;
+		else
+			die(_("A branch named '%s' already exists."),
+				ref->buf + strlen("refs/heads/"));
+	}
 
 	head = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
-	if (!is_bare_repository() && head && !strcmp(head, ref->buf))
-		die(_("Cannot force update the current branch."));
+	if (!is_bare_repository() && head && !strcmp(head, ref->buf)) {
+		if (dont_fail)
+			return CANNOT_FORCE_UPDATE_CURRENT_BRANCH;
+		else
+			die(_("Cannot force update the current branch."));
+	}
 
-	return 1;
+	return BRANCH_EXISTS;
 }
 
 static int check_tracking_branch(struct remote *remote, void *cb_data)
@@ -259,8 +269,8 @@ void create_branch(const char *name, const char *start_name,
 		explicit_tracking = 1;
 
 	if ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok)
-	    ? validate_branchname(name, &ref)
-	    : validate_new_branchname(name, &ref, force)) {
+	    ? validate_branchname(name, &ref, 0)
+	    : validate_new_branchname(name, &ref, force, 0)) {
 		if (!force)
 			dont_change_ref = 1;
 		else
diff --git a/branch.h b/branch.h
index 85052628b..0c178ec5a 100644
--- a/branch.h
+++ b/branch.h
@@ -27,20 +27,58 @@ void create_branch(const char *name, const char *start_name,
 		   enum branch_track track, int force, int clobber_head_ok,
 		   int reflog, int quiet);
 
-/*
- * Check if 'name' can be a valid name for a branch; die otherwise.
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
-extern int validate_branchname(const char *name, struct strbuf *ref);
+enum branch_validation_result {
+	/* Flags that convey that validation FAILED */
+	BRANCH_EXISTS_NO_FORCE = -3,
+	CANNOT_FORCE_UPDATE_CURRENT_BRANCH,
+	INVALID_BRANCH_NAME,
+	/* Flags that convey that validation SUCCEEDED */
+	BRANCH_DOESNT_EXIST = 0,
+	BRANCH_EXISTS = 1,
+};
 
 /*
- * Check if a branch 'name' can be created as a new branch; die otherwise.
- * 'force' can be used when it is OK for the named branch already exists.
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
+ * Check if 'name' can be a valid name for a branch; die otherwise.
+ *
+ *   - name is the new branch name
+ *
+ *   - ref is used to return the full refname for the branch
+ *
+ * The return values have the following meaning,
+ *
+ *   - If dont_fail is 0, the function dies in case of failure and returns flags of
+ *     'branch_validation_result' that indicate status of the given branch. The positive
+ *     non-zero flag implies that the branch exists.
+ *
+ *   - If dont_fail is 1, the function doesn't die in case of failure but returns flags
+ *     of 'branch_validaton_result' that specify the reason for failure. The behaviour in case of
+ *     success is same as above.
+ *
  */
-extern int validate_new_branchname(const char *name, struct strbuf *ref, int force);
+extern enum branch_validation_result validate_branchname(const char *name, struct strbuf *ref, unsigned dont_fail);
+
+/*
+ * Check if a branch 'name' can be created as a new branch.
+ *
+ *   - name is the new branch name
+ *
+ *   - ref is used to return the full refname for the branch
+ *
+ *   - force can be used when it is OK if the named branch already exists.
+ *     the currently checkout branch; with 'shouldnt_exist', it has no effect.
+ *
+ * The return values have the following meaning,
+ *
+ *   - If dont_fail is 0, the function dies in case of failure and returns flags of
+ *     'branch_validation_result' that indicate that convey status of given branch. The positive
+ *     non-zero flag implies that the branch can be force updated.
+ *
+ *   - If dont_fail is 1, the function doesn't die in case of failure but returns flags
+ *     of 'branch_validaton_result' that specify the reason for failure. The behaviour in case of
+ *     success is same as above.
+ *
+ */
+extern enum branch_validation_result validate_new_branchname(const char *name, struct strbuf *ref, int force, unsigned dont_fail);
 
 /*
  * Remove information about the state of working on the current
diff --git a/builtin/branch.c b/builtin/branch.c
index df06ac968..7018e5d75 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -487,9 +487,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 	 * cause the worktree to become inconsistent with HEAD, so allow it.
 	 */
 	if (!strcmp(oldname, newname))
-		validate_branchname(newname, &newref);
+		validate_branchname(newname, &newref, 0);
 	else
-		validate_new_branchname(newname, &newref, force);
+		validate_new_branchname(newname, &newref, force, 0);
 
 	reject_rebase_or_bisect_branch(oldref.buf);
 
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5c34a9a0d..4adab3814 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1288,10 +1288,11 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		struct strbuf buf = STRBUF_INIT;
 
 		if (opts.new_branch_force)
-			opts.branch_exists = validate_branchname(opts.new_branch, &buf);
+			opts.branch_exists = validate_branchname(opts.new_branch, &buf, 0);
 		else
 			opts.branch_exists =
-				validate_new_branchname(opts.new_branch, &buf, 0);
+				validate_new_branchname(opts.new_branch, &buf, 0, 0);
+
 		strbuf_release(&buf);
 	}
 
-- 
2.15.0.461.gf957c703b.dirty

[RFC PATCH v3 4/4] builtin/branch: give more useful error messages when renaming

[Kaartic Sivaraam]

From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>

When trying to rename an inexistent branch to with a name of a branch
that already exists the rename failed specifying the new branch name
exists rather than specifying that the branch trying to be renamed
doesn't exist.

    $ git branch -m tset master
    fatal: A branch named 'master' already exists.

It's conventional to report that 'tset' doesn't exist rather than
reporting that 'master' exists, the same way the 'mv' command does.

    (hypothetical)
    $ git branch -m tset master
    fatal: branch 'tset' doesn't exist.

That has the problem that the error about an existing branch is shown
only after the user corrects the error about inexistent branch.

    $ git branch -m test master
    fatal: A branch named 'master' already exists.

This isn't useful either because the user would have corrected this error in
a single go if he had been told this alongside the first error. So, give
more useful error messages by giving errors about old branch name and new
branch name at the same time. This is possible as the branch name validation
functions now return the reason they were about to die, when requested.

    $ git branch -m tset master
    fatal: branch 'tset' doesn't exist, and branch 'master' already exists

Note: Thanks to the strbuf API that made it possible to easily construct
the composite error message strings!

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 builtin/branch.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 7018e5d75..c2bbf8c3d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -458,11 +458,42 @@ static void reject_rebase_or_bisect_branch(const char *target)
 	free_worktrees(worktrees);
 }
 
+static void get_error_msg(struct strbuf* error_msg, const char* oldname, unsigned old_branch_exists,
+			  const char* newname, enum branch_validation_result res)
+{
+	const char* connector_string = _(", and ");
+
+	if (!old_branch_exists) {
+		strbuf_addf(error_msg, _("branch '%s' doesn't exist"), oldname);
+	}
+
+	switch (res) {
+		case BRANCH_EXISTS_NO_FORCE:
+			strbuf_addf(error_msg, "%s", (!old_branch_exists) ? connector_string : "");
+			strbuf_addf(error_msg,_("branch '%s' already exists"), newname);
+			break;
+		case CANNOT_FORCE_UPDATE_CURRENT_BRANCH:
+			strbuf_addf(error_msg, "%s", (!old_branch_exists) ? connector_string : "");
+			strbuf_addstr(error_msg, _("cannot force update the current branch"));
+			break;
+		case INVALID_BRANCH_NAME:
+			strbuf_addf(error_msg, "%s", (!old_branch_exists) ? connector_string : "");
+			strbuf_addf(error_msg, _("branch name '%s' is invalid"), newname);
+			break;
+		/* not necessary to handle success cases */
+		case BRANCH_EXISTS:
+		case BRANCH_DOESNT_EXIST:
+			break;
+	}
+}
+
 static void copy_or_rename_branch(const char *oldname, const char *newname, int copy, int force)
 {
 	struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
 	struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
 	int recovery = 0;
+	struct strbuf error_msg = STRBUF_INIT, empty = STRBUF_INIT;
+	enum branch_validation_result res;
 
 	if (!oldname) {
 		if (copy)
@@ -471,15 +502,13 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 			die(_("cannot rename the current branch while not on any."));
 	}
 
-	if (strbuf_check_branch_ref(&oldref, oldname)) {
+	if (strbuf_check_branch_ref(&oldref, oldname) && ref_exists(oldref.buf))
+	{
 		/*
 		 * Bad name --- this could be an attempt to rename a
 		 * ref that we used to allow to be created by accident.
 		 */
-		if (ref_exists(oldref.buf))
-			recovery = 1;
-		else
-			die(_("Invalid branch name: '%s'"), oldname);
+		recovery = 1;
 	}
 
 	/*
@@ -487,9 +516,13 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 	 * cause the worktree to become inconsistent with HEAD, so allow it.
 	 */
 	if (!strcmp(oldname, newname))
-		validate_branchname(newname, &newref, 0);
+		res = validate_branchname(newname, &newref, 1);
 	else
-		validate_new_branchname(newname, &newref, force, 0);
+		res = validate_new_branchname(newname, &newref, force, 1);
+
+	get_error_msg(&error_msg, oldname, ref_exists(oldref.buf), newname, res);
+	if (strbuf_cmp(&error_msg, &empty))
+		die("%s", error_msg.buf);
 
 	reject_rebase_or_bisect_branch(oldref.buf);
 
@@ -530,6 +563,8 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 		die(_("Branch is copied, but update of config-file failed"));
 	strbuf_release(&oldsection);
 	strbuf_release(&newsection);
+	strbuf_release(&error_msg);
+	strbuf_release(&empty);
 }
 
 static GIT_PATH_FUNC(edit_description, "EDIT_DESCRIPTION")
-- 
2.15.0.461.gf957c703b.dirty

Re: [RFC PATCH v3 0/4] give more useful error messages while renaming branch

[Kaartic Sivaraam]

Sorry, ignore this mails. I actually have re-sent it to the correct
thread.
 
-- 
Kaartic

[RFC PATCH v2 0/5] Give more useful error messages when renaming a branch

[Kaartic Sivaraam]

    [1/5] and [2/5] - cleanup patches
    [3/5] - refactor of a function that seemed a little unintuitive
    [4/5] - just a preparatory step adding part of the logic
    [5/5] - main patch that tries to improve the error messages

Changes in v2:

    - dropped [1/5] of v1
    
    [1/5]

        - incroporated suggestions of Junio

    [2/5]

        - new in this series
        - just a little thing I thought could improve things

    [3/5]

        - used the name 'validate_branch_creation' for the refactor
          instead of 'validate_branch_update'

        - changed the parameter names

    [5/5]

        - In v1 I added the connector string and removed it when it wasn't 
          necessary. It's not possible to do that as that string has to be
          translated too and it won't have the same number of characters in
          other languages. So, I switched to adding the connector only when
          needed. Though this seems to introduce some redundancy, I find it
          to be the only reliable way.          

        - tweaked commit message a little as suggested by Stefan

There is no change in the error messages themselves. So the sample input
output interaction sent for v1 still holds.

This series applies on top of 'master' and can be found in my fork[1].
In case you were wondering, the Travis-CI tests did pass.

[1]: https://github.com/sivaraam/git/tree/work/branch-move-revamp
[2]: https://travis-ci.org/sivaraam/git/builds/279199977

Kaartic Sivaraam (5):
  branch: improve documentation and naming of certain parameters
  branch: re-order function arguments to group related arguments
  branch: cleanup branch name validation
  branch: introduce dont_fail parameter for create validation
  builtin/branch: give more useful error messages when renaming

 branch.c           | 69 +++++++++++++++++++++++++++++++++++++++---------------
 branch.h           | 53 ++++++++++++++++++++++++++++++-----------
 builtin/branch.c   | 43 ++++++++++++++++++++++++++++------
 builtin/checkout.c |  8 +++----
 t/t3200-branch.sh  |  4 ++++
 5 files changed, 133 insertions(+), 44 deletions(-)

-- 
2.14.1.935.ge2b2bcd8a

[RFC PATCH v3 0/4] give more useful error messages while renaming branch

[Kaartic Sivaraam]

In builtin/branch, the error messages weren't handled directly by the branch
renaming function and was left to the other function. Though this avoids
redundancy this gave unclear error messages in some cases. So, make builtin/branch
give more useful error messages.

Changes in v3:

	Incorporated suggestions from v2 to improve code and commit message. To
	be more precise about the code part,

	In 2/4 slightly re-ordered the parameters to move the flag parameters to
	the end.

	In 3/4, changed the return type of the branchname validation functions to
	be the enum (whose values they return) as suggested by Stefan.

	Dropped the PATCH 3/5 of v2 as there was another series[1] that did the
	refactor and got merged to 'next'. I have now re-rolled the series over
	'next' [pointing at 273055501 (Sync with master, 2017-10-24)].
 
	This has made the code in 3/4 a little clumsy (at least to me) as I
	tried to achieve to achieve what the previous patches did with the new
	validate*_branchname functionS. Let me know, if it looks too bad.

So this could go on top of 'next' without any conflicts but in case I
missed something, let me know. The series could be found in my fork[2].


Any feedback welcome.

Thanks,
Kaartic

[1] : https://public-inbox.org/git/20171013051132.3973-1-gitster@pobox.com

[2] : https://github.com/sivaraam/git/tree/work/branch-revamp


Kaartic Sivaraam (4):
  branch: improve documentation and naming of 'create_branch()'
  branch: re-order function arguments to group related arguments
  branch: introduce dont_fail parameter for branchname validation
  builtin/branch: give more useful error messages when renaming

 branch.c           | 63 ++++++++++++++++++++++++++++++------------------------
 branch.h           | 57 ++++++++++++++++++++++++++++++++++++++----------
 builtin/branch.c   | 49 ++++++++++++++++++++++++++++++++++--------
 builtin/checkout.c | 11 +++++-----
 4 files changed, 127 insertions(+), 53 deletions(-)

-- 
2.15.0.rc2.401.g3db9995f9