From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got: refactor choose_patch
To: gameoftrees@openbsd.org
Date: Sun, 28 Apr 2024 21:44:25 +0200

On Sun, Apr 28, 2024 at 03:10:18PM +0000, Lucas Gabriel Vuotto wrote:
> + prompt:
> +	err = show_change(status, path, patch_file, n, nchanges,
> +	    a->action);
> +	if (err)
> +		return err;

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.

> +
> +	linelen = getline(&line, &linesize, fp);
> +	if (linelen == -1) {
> +		free(line);
> +		if (ferror(fp))
> +			return got_error_from_errno("getline");
> +		if (feof(fp))
> +			return got_error(GOT_ERR_EOF);
> +		return NULL;
> +	}
> +
> +	nl = strchr(line, '\n');
> +	if (nl)
> +		*nl = '\0';
> +	if (strlen(line) != 1 ||
> +	    !strchr(CHOOSE_PATCH_VALID_RESPONSES, line[0])) {
> +		printf("invalid response '%s'\n", line);
> +		if (interactive)
> +			goto prompt;
> +	}
> +
> +	if (interactive) {
> +		if (status != GOT_STATUS_MODIFY && line[0] == 'q') {
>  			printf("invalid response '%s'\n", line);
> -		free(line);
> -		return NULL;
> -	}
> -
> -	while (resp != 'y' && resp != 'n' && resp != 'q') {
> -		err = show_change(status, path, patch_file, n, nchanges,
> -		    a->action);
> -		if (err)
> -			return err;
> -		resp = getchar();
> -		if (resp == '\n')
> -			resp = getchar();
> -		if (status == GOT_STATUS_MODIFY) {
> -			if (resp != 'y' && resp != 'n' && resp != 'q') {
> -				printf("invalid response '%c'\n", resp);
> -				resp = ' ';
> -			}
> -		} else if (resp != 'y' && resp != 'n') {
> -				printf("invalid response '%c'\n", resp);
> -				resp = ' ';
> +			goto prompt;
>  		}
>  	}