From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got: refactor choose_patch
To: gameoftrees@openbsd.org
Date: Mon, 29 Apr 2024 23:03:27 +0200

On Mon, Apr 29, 2024 at 06:01:20AM +0000, Lucas Gabriel Vuotto wrote:
> On Sun, Apr 28, 2024 at 09:44:25PM GMT, Stefan Sperling wrote:
> > Please try to avoid 'goto' for control-flow purposes, other than
> > the usual 'goto error' cleanup case. It is very rare that such
> > jumps cannot be expressed as a loop that reads better.
> 
> Fair. I avoided it as I tend to find loops with break in their body
> rather confusing. Here's an updated and rebased version.

Thanks, committed.

I propose two tweaks on top:

1) I did not realize that interactive and -F mode differed in their
acceptance of the 'q' response in places. I would like to unify
this behaviour. I'm not even sure why we don't accept 'q' for
additions and deletions, but I'll leave that questions for later.

2) Add a default case to the switch statement at the bottom, just
in case the code futher up gets tweaked again such that the validity
check using strchr() will be skipped in some edge case.

diff /home/stsp/src/got
commit - e6370d593b348b23199788e4b0d5a23f14dde15e
path + /home/stsp/src/got
blob - 92d7198e1d1093296cd4b3a1cc0d49a6de2c3de5
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -8683,11 +8683,10 @@ choose_patch(int *choice, void *arg, unsigned char sta
 				continue;
 		}
 
-		if (interactive) {
-			if (status != GOT_STATUS_MODIFY && line[0] == 'q') {
-				printf("invalid response '%s'\n", line);
-				continue;
-			}
+		/* ADD and DELETE do not accept 'q' response currently. */
+		if (status != GOT_STATUS_MODIFY && line[0] == 'q') {
+			printf("invalid response '%s'\n", line);
+			continue;
 		}
 
 		break;
@@ -8704,6 +8703,10 @@ choose_patch(int *choice, void *arg, unsigned char sta
 		if (status == GOT_STATUS_MODIFY)
 			*choice = GOT_PATCH_CHOICE_QUIT;
 		break;
+	default:
+		printf("invalid response '%s'\n", line);
+		free(line);
+		return NULL;
 	}
 
 	if (!interactive)