diff mbox series

[08/11] bisect--helper: using `--bisect-state` without an argument is a bug

Message ID 1b14ed3d7976b8fe4149822dcbb32343e0554a38.1643328752.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 Jan. 28, 2022, 12:12 a.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

The `bisect--helper` command is not expected to be used directly by the
user. Therefore, it is a bug if it receives no argument to the
`--bisect-state` command mode, not a user error. Which means that we
need to call `BUG()` instead of `die()`.

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

Comments

Ævar Arnfjörð Bjarmason Jan. 29, 2022, 7:04 a.m. UTC | #1
On Fri, Jan 28 2022, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The `bisect--helper` command is not expected to be used directly by the
> user. Therefore, it is a bug if it receives no argument to the
> `--bisect-state` command mode, not a user error. Which means that we
> need to call `BUG()` instead of `die()`.

I found this commit message confusing with 07/11. I.e. won't this be
"git bisect" directly make this BUG() a bug then?

But no, it won't, because this being a bug has nothing to do with it
being a helper at all. after 07/11, but that it'll be called from an
implicit "if (!argc)" in cmd_bisect(), no?

I think I've either misread that, or this commit message is very
misleading.
Elijah Newren Feb. 9, 2022, 12:26 a.m. UTC | #2
On Fri, Jan 28, 2022 at 3:30 PM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The `bisect--helper` command is not expected to be used directly by the
> user. Therefore, it is a bug if it receives no argument to the
> `--bisect-state` command mode, not a user error. Which means that we
> need to call `BUG()` instead of `die()`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/bisect--helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 4450305c81c..009c919a989 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -872,7 +872,7 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a
>         struct oid_array revs = OID_ARRAY_INIT;
>
>         if (!argc)
> -               return error(_("Please call `--bisect-state` with at least one argument"));
> +               BUG("bisect_state() called without argument");
>
>         if (bisect_autostart(terms))
>                 return BISECT_FAILED;
> --
> gitgitgadget

Makes sense.
diff mbox series

Patch

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 4450305c81c..009c919a989 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -872,7 +872,7 @@  static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a
 	struct oid_array revs = OID_ARRAY_INIT;
 
 	if (!argc)
-		return error(_("Please call `--bisect-state` with at least one argument"));
+		BUG("bisect_state() called without argument");
 
 	if (bisect_autostart(terms))
 		return BISECT_FAILED;