[PATCH] branch: reject -D/-d without branch name

Subsystems: the rest

STALE3735d

10 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] branch: reject -D/-d without branch name

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:55:53

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/branch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..1d3e842 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -837,7 +837,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		colopts = 0;
 	}
 
-	if (delete)
+	if (delete && argc)
 		return delete_branches(argc, argv, delete > 1, kinds, quiet);
 	else if (list) {
 		int ret = print_ref_list(kinds, detached, verbose, abbrev,

-- 
1.8.1.1.380.g782aa97

[PATCH v2 1/3] branch: reject -D/-d without branch name

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:55:53

---
 builtin/branch.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..50fcacc 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -837,9 +837,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		colopts = 0;
 	}
 
-	if (delete)
+	if (delete) {
+		if (!argc)
+			die(_("branch name required"));
 		return delete_branches(argc, argv, delete > 1, kinds, quiet);
-	else if (list) {
+	} else if (list) {
 		int ret = print_ref_list(kinds, detached, verbose, abbrev,
 					 with_commit, argv);
 		print_columns(&output, colopts, NULL);

-- 
1.8.0.rc2.23.g1fb49df

[PATCH v2 2/3] branch: give a more helpful message on redundant arguments

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:55:53

While at there, do not stop user from editing a branch description
when the unrelated HEAD is detached.
---
 builtin/branch.c  | 12 ++++++------
 t/t3200-branch.sh |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 50fcacc..ca61c5b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -852,14 +852,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		const char *branch_name;
 		struct strbuf branch_ref = STRBUF_INIT;
 
-		if (detached)
-			die("Cannot give description to detached HEAD");
-		if (!argc)
+		if (!argc) {
+			if (detached)
+				die("Cannot give description to detached HEAD");
 			branch_name = head;
-		else if (argc == 1)
+		} else if (argc == 1)
 			branch_name = argv[0];
 		else
-			usage_with_options(builtin_branch_usage, options);
+			die(_("cannot edit description of more than one branch"));
 
 		strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
 		if (!ref_exists(branch_ref.buf)) {

@@ -881,7 +881,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		else if (argc == 2)
 			rename_branch(argv[0], argv[1], rename > 1);
 		else
-			usage_with_options(builtin_branch_usage, options);
+			die(_("too many branches for a rename operation"));
 	} else if (new_upstream) {
 		struct branch *branch = branch_get(argv[0]);

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 80e6be3..f3e0e4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh

@@ -73,8 +73,8 @@ test_expect_success \
 
 test_expect_success \
     'git branch -m dumps usage' \
-       'test_expect_code 129 git branch -m 2>err &&
-	test_i18ngrep "[Uu]sage: git branch" err'
+       'test_expect_code 128 git branch -m 2>err &&
+	test_i18ngrep "too many branches for a rename operation" err'
 
 test_expect_success \
     'git branch -m m m/m should work' \

-- 
1.8.0.rc2.23.g1fb49df

[PATCH v2 3/3] branch: mark more strings for translation

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:55:53

---
 builtin/branch.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ca61c5b..597b578 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -466,7 +466,7 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item,
 			     int verbose, int abbrev)
 {
 	struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
-	const char *sub = " **** invalid ref ****";
+	const char *sub = _(" **** invalid ref ****");
 	struct commit *commit = item->commit;
 
 	if (commit && !parse_commit(commit)) {

@@ -590,7 +590,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
 		struct commit *filter;
 		filter = lookup_commit_reference_gently(merge_filter_ref, 0);
 		if (!filter)
-			die("object '%s' does not point to a commit",
+			die(_("object '%s' does not point to a commit"),
 			    sha1_to_hex(merge_filter_ref));
 
 		filter->object.flags |= UNINTERESTING;

@@ -854,7 +854,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 		if (!argc) {
 			if (detached)
-				die("Cannot give description to detached HEAD");
+				die(_("Cannot give description to detached HEAD"));
 			branch_name = head;
 		} else if (argc == 1)
 			branch_name = argv[0];

@@ -866,10 +866,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			strbuf_release(&branch_ref);
 
 			if (!argc)
-				return error("No commit on branch '%s' yet.",
+				return error(_("No commit on branch '%s' yet."),
 					     branch_name);
 			else
-				return error("No such branch '%s'.", branch_name);
+				return error(_("No branch named '%s'."),
+					     branch_name);
 		}
 		strbuf_release(&branch_ref);

-- 
1.8.0.rc2.23.g1fb49df

Re: [PATCH v2 2/3] branch: give a more helpful message on redundant arguments

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:55:55

Nguyễn Thái Ngọc Duy wrote:

quoted hunk

--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -852,14 +852,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		const char *branch_name;
 		struct strbuf branch_ref = STRBUF_INIT;
 
-		if (detached)
-			die("Cannot give description to detached HEAD");
-		if (!argc)
+		if (!argc) {
+			if (detached)
+				die("Cannot give description to detached HEAD");

Good catch.  Shouldn't this bugfix be a separate patch, so it can also
be included in maint?

Re: [PATCH v2 3/3] branch: mark more strings for translation

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:55:55

Nguyễn Thái Ngọc Duy wrote:

quoted hunk

--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -466,7 +466,7 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item,
 			     int verbose, int abbrev)
 {
 	struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
-	const char *sub = " **** invalid ref ****";
+	const char *sub = _(" **** invalid ref ****");

This worried me for a second --- is it an actual message that gets
emitted, a placeholder used only in code, or some combination of
the two?

Luckily it really is just a message (or rather, a value for the commit
message column in the " f7c0c00 [ahead 58, behind 197] vcs-svn: drop
obj_pool.h" message).

For what it's worth, assuming this passes tests,
Reviewed-by: Jonathan Nieder <redacted>

[PATCH v3 3/4] branch: give a more helpful message on redundant arguments

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:55:55

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/branch.c  | 4 ++--
 t/t3200-branch.sh | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 30c4545..ca61c5b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -859,7 +859,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		} else if (argc == 1)
 			branch_name = argv[0];
 		else
-			usage_with_options(builtin_branch_usage, options);
+			die(_("cannot edit description of more than one branch"));
 
 		strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
 		if (!ref_exists(branch_ref.buf)) {

@@ -881,7 +881,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		else if (argc == 2)
 			rename_branch(argv[0], argv[1], rename > 1);
 		else
-			usage_with_options(builtin_branch_usage, options);
+			die(_("too many branches for a rename operation"));
 	} else if (new_upstream) {
 		struct branch *branch = branch_get(argv[0]);

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 80e6be3..f3e0e4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh

@@ -73,8 +73,8 @@ test_expect_success \
 
 test_expect_success \
     'git branch -m dumps usage' \
-       'test_expect_code 129 git branch -m 2>err &&
-	test_i18ngrep "[Uu]sage: git branch" err'
+       'test_expect_code 128 git branch -m 2>err &&
+	test_i18ngrep "too many branches for a rename operation" err'
 
 test_expect_success \
     'git branch -m m m/m should work' \

-- 
1.8.1.1.459.g5970e58

[PATCH v3 4/4] branch: mark more strings for translation

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:55:55

Reviewed-by: Jonathan Nieder <redacted>
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 On Sun, Jan 27, 2013 at 6:55 PM, Jonathan Nieder [off-list ref] wrote:
 > For what it's worth, assuming this passes tests,

 It passes the tests. Although I doubt the tests are written to catch
 this.

 builtin/branch.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ca61c5b..597b578 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -466,7 +466,7 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item,
 			     int verbose, int abbrev)
 {
 	struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
-	const char *sub = " **** invalid ref ****";
+	const char *sub = _(" **** invalid ref ****");
 	struct commit *commit = item->commit;
 
 	if (commit && !parse_commit(commit)) {

@@ -590,7 +590,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
 		struct commit *filter;
 		filter = lookup_commit_reference_gently(merge_filter_ref, 0);
 		if (!filter)
-			die("object '%s' does not point to a commit",
+			die(_("object '%s' does not point to a commit"),
 			    sha1_to_hex(merge_filter_ref));
 
 		filter->object.flags |= UNINTERESTING;

@@ -854,7 +854,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 		if (!argc) {
 			if (detached)
-				die("Cannot give description to detached HEAD");
+				die(_("Cannot give description to detached HEAD"));
 			branch_name = head;
 		} else if (argc == 1)
 			branch_name = argv[0];

@@ -866,10 +866,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			strbuf_release(&branch_ref);
 
 			if (!argc)
-				return error("No commit on branch '%s' yet.",
+				return error(_("No commit on branch '%s' yet."),
 					     branch_name);
 			else
-				return error("No such branch '%s'.", branch_name);
+				return error(_("No branch named '%s'."),
+					     branch_name);
 		}
 		strbuf_release(&branch_ref);

-- 
1.8.1.1.459.g5970e58

[PATCH v3 1/4] branch: no detached HEAD check when editing another branch's description

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:55:55

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Split out from v3's 2/3 as Jonathan suggested.

 builtin/branch.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..ea6498b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -850,11 +850,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		const char *branch_name;
 		struct strbuf branch_ref = STRBUF_INIT;
 
-		if (detached)
-			die("Cannot give description to detached HEAD");
-		if (!argc)
+		if (!argc) {
+			if (detached)
+				die("Cannot give description to detached HEAD");
 			branch_name = head;
-		else if (argc == 1)
+		} else if (argc == 1)
 			branch_name = argv[0];
 		else
 			usage_with_options(builtin_branch_usage, options);

-- 
1.8.1.1.459.g5970e58

[PATCH v3 2/4] branch: reject -D/-d without branch name

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:55:55

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/branch.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ea6498b..30c4545 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -837,9 +837,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		colopts = 0;
 	}
 
-	if (delete)
+	if (delete) {
+		if (!argc)
+			die(_("branch name required"));
 		return delete_branches(argc, argv, delete > 1, kinds, quiet);
-	else if (list) {
+	} else if (list) {
 		int ret = print_ref_list(kinds, detached, verbose, abbrev,
 					 with_commit, argv);
 		print_columns(&output, colopts, NULL);

-- 
1.8.1.1.459.g5970e58

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help