Re: [PATCH/RFC v4 2/5] sha1_name: tweak @{-N} lookup

[Junio C Hamano <gitster@pobox.com>]

Thomas Rast <trast@student.ethz.ch> writes:

> Have the lookup only look at "interesting" checkouts, meaning those
> that tell you "Already on ..." don't count even though they also cause
> a reflog entry.
>
> Let interpret_nth_last_branch() return the number of characters
> parsed, so that git-checkout can verify that the branch spec was
> @{-N}, not @{-1}^2 or something like that.  (The latter will be added
> later.)

Thanks; you seem to have handled the issues I pointed out in response to
my own weatherbaloon patch.  I think it is probably better to squash the
first two (and you take the authorship).

> diff --git a/sha1_name.c b/sha1_name.c
> index 6377264..34e39db 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -685,29 +685,28 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
>  				  const char *message, void *cb_data)
>  {
>  	struct grab_nth_branch_switch_cbdata *cb = cb_data;
> -	const char *match = NULL;
> -
> -	if (!prefixcmp(message, "checkout: moving to "))
> -		match = message + strlen("checkout: moving to ");
> -	else if (!prefixcmp(message, "checkout: moving from ")) {
> -		const char *cp = message + strlen("checkout: moving from ");
> -		if ((cp = strstr(cp, " to ")) != NULL) {
> -			match = cp + 4;
> -		}
> +	const char *match = NULL, *target = NULL;
> +	size_t len;
> +
> +	if (!prefixcmp(message, "checkout: moving from ")) {
> +		match = message + strlen("checkout: moving from ");
> +		if ((target = strstr(match, " to ")) != NULL)
> +			target += 4;
>  	}

This drops support for older reflog records, but I think it would be Ok.
This "N-th" support is really meant to be for small number of N anyway.

> -	if (--cb->nth <= 0) {
> -		size_t len = strlen(match);
> -		while (match[len-1] == '\n')
> -			len--;
> +	if (cb->nth-- <= 0) {
>  		strbuf_reset(cb->buf);
>  		strbuf_add(cb->buf, match, len);
>  		return 1;

Hmm, did I have an off-by-one I did not notice? ;-)

>  int interpret_nth_last_branch(const char *name, struct strbuf *buf)
>  {
> -	int nth, i;
> +	int nth;
>  	struct grab_nth_branch_switch_cbdata cb;
> +	const char *brace;
> +	char *num_end;
>  
>  	if (name[0] != '@' || name[1] != '{' || name[2] != '-')
>  		return -1;
> -	for (i = 3, nth = 0; name[i] && name[i] != '}'; i++) {
> -		char ch = name[i];
> -		if ('0' <= ch && ch <= '9')
> -			nth = nth * 10 + ch - '0';
> -		else
> -			return -1;
> -	}
> -	if (nth < 0 || 10 <= nth)

The removal of "limit to reasonably small recent N" I somewhat have
reservations on, but I think we can later re-add something based on
configuration variable if we need to.