Re: [PATCH/RFC v4 2/5] sha1_name: tweak @{-N} lookup
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 18, 2009, 00:54 UTC
- Message-ID
- <7v4ozx5t8b.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1232208597-29249-3-git-send-email-trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
Show 8 quoted lines
> 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).
Show 25 quoted lines
> 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.
Show 8 quoted lines
> - 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? ;-)
Show 18 quoted lines
> 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.