diff mbox series

[v4,11/16] bisect--helper: return only correct exit codes in `cmd_*()`

Message ID ce508583e455a1dbb7620a238edb11dae195f00d.1656354677.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Finish converting git bisect into a built-in | expand

Commit Message

Johannes Schindelin June 27, 2022, 6:31 p.m. UTC 
From: Johannes Schindelin <johannes.schindelin@gmx.de>

Exit codes cannot be negative, but `error()` returns -1.

Let's just go with the common pattern and call `die()` in
`cmd_bisect__helper()` when incorrect arguments were detected.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/bisect--helper.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Ævar Arnfjörð Bjarmason June 27, 2022, 8:09 p.m. UTC | #1 
On Mon, Jun 27 2022, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Exit codes cannot be negative, but `error()` returns -1.

That's good, but...

> Let's just go with the common pattern and call `die()` in
> `cmd_bisect__helper()` when incorrect arguments were detected.

...the common and correct pattern on bad options is to return 129, not
128, which using use usage_msg_opt() instead of die() here would give
you for free. I think this should use that instead.
diff mbox series

Patch

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 22099b9ebe2..86195058cd5 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -1327,7 +1327,7 @@  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		break;
 	case BISECT_TERMS:
 		if (argc > 1)
-			return error(_("--bisect-terms requires 0 or 1 argument"));
+			die(_("--bisect-terms requires 0 or 1 argument"));
 		res = bisect_terms(&terms, argc == 1 ? argv[0] : NULL);
 		break;
 	case BISECT_SKIP:
@@ -1337,13 +1337,13 @@  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		break;
 	case BISECT_NEXT:
 		if (argc)
-			return error(_("--bisect-next requires 0 arguments"));
+			die(_("--bisect-next requires 0 arguments"));
 		get_terms(&terms);
 		res = bisect_next(&terms, prefix);
 		break;
 	case BISECT_RESET:
 		if (argc > 1)
-			return error(_("--bisect-reset requires either no argument or a commit"));
+			die(_("--bisect-reset requires either no argument or a commit"));
 		res = bisect_reset(argc ? argv[0] : NULL);
 		break;
 	case BISECT_VISUALIZE:
@@ -1352,18 +1352,18 @@  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		break;
 	case BISECT_REPLAY:
 		if (argc != 1)
-			return error(_("no logfile given"));
+			die(_("no logfile given"));
 		set_terms(&terms, "bad", "good");
 		res = bisect_replay(&terms, argv[0]);
 		break;
 	case BISECT_LOG:
 		if (argc)
-			return error(_("--bisect-log requires 0 arguments"));
+			die(_("--bisect-log requires 0 arguments"));
 		res = bisect_log();
 		break;
 	case BISECT_RUN:
 		if (!argc)
-			return error(_("bisect run failed: no command provided."));
+			die(_("bisect run failed: no command provided."));
 		get_terms(&terms);
 		res = bisect_run(&terms, argv, argc);
 		break;