Subject: Re: [PATCH v2] rebase: clarify conditionals in todo_list_to_strbuf()

From: Felipe Contreras <[email-masked]>
Date: Tue, 02 May 2023 12:51:37 -0600

Oswald Buddenhagen wrote:
> Make it obvious that the two conditional branches are mutually
> exclusive. This makes it easier to comprehend and optimize.
> 
> As a drive-by, remove a pair of unnecessary braces.
> 
> Signed-off-by: Oswald Buddenhagen <[email-masked]>
> ---
> v2:
> - slightly more verbose commit message
> ---
>  sequencer.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 3be23d7ca2..9169876441 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -5868,12 +5868,9 @@ static void todo_list_to_strbuf(struct repository *r, struct todo_list *todo_lis
>  			if (item->command == TODO_FIXUP) {
>  				if (item->flags & TODO_EDIT_FIXUP_MSG)
>  					strbuf_addstr(buf, " -c");
> -				else if (item->flags & TODO_REPLACE_FIXUP_MSG) {
> +				else if (item->flags & TODO_REPLACE_FIXUP_MSG)
>  					strbuf_addstr(buf, " -C");
> -				}
> -			}
> -
> -			if (item->command == TODO_MERGE) {
> +			} else if (item->command == TODO_MERGE) {
>  				if (item->flags & TODO_EDIT_MERGE_MSG)
>  					strbuf_addstr(buf, " -c");
>  				else
> -- 

FWIW makes total sense to me and does make the code easier to
comprehend.

Reviewed-by: Felipe Contreras <[email-masked]>

-- 
Felipe Contreras