"Lidong Yan via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Lidong Yan <502024330056@xxxxxxxxxxxxxxxx>
>
> In sequencer.c, caller only pass TODO_SQUASH or TODO_FIXUP to
> update_squash_messages(), any other command passed in should be
> considered as BUG. Thus I think `return error('unknown command')`
> should be replaced as `BUG('unknown command')`.
Yup. The only caller has
else if (is_fixup(command)) {
if (update_squash_messages(r, command, ...) {
and is_fixup() is confusingly [*] defined to return true only when
the command is one of these two values.
[Footnote]
* And a similarly named is_fixup_flag() only accepts TODO_FIXUP and
never TODO_SQUASH. Both of these helpers probably need to be
renamed.
> diff --git a/sequencer.c b/sequencer.c
> index b5c4043757e..3cd0dd3434e 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2139,7 +2139,7 @@ static int update_squash_messages(struct repository *r,
> strbuf_add_commented_lines(&buf, body, strlen(body),
> comment_line_str);
> } else
> - return error(_("unknown command: %d"), command);
> + BUG(_("unknown command: %d"), command);
> repo_unuse_commit_buffer(r, commit, message);
BUG() is not end-user facing but programmer facing, and we do not
use _("...") in them. I see a few existing violators that need to
be corrected.
OK. Or
if (!is_fixup(command))
BUG("not a FIXUP or SQUASH %d", command);
at the very beginning of the function?
