Re: [PATCH v5 3/4] status: give more information during rebase -i

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jul 1, 2015, 21:06 UTC
Message-ID: <vpqk2ujoadw.fsf@anie.imag.fr>
In-Reply-To: <xmqq4mlnc0lr.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 14 quoted lines

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> +		strbuf_trim(split[1]);
>> +		if (!get_sha1(split[1]->buf, sha1)) {
>> +			abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
>> +			strbuf_reset(split[1]);
>> +			strbuf_addf(split[1], "%s ", abbrev);
>> +		}
>
> ... else?
>
> That is, "we thought there would be a full SHA-1, but it turns out
> that there wasn't, so we keep split[1] as-is" would need to add the
> space back, no?

Right.

Show 7 quoted lines

> Perhaps be more strict and do this instead (without leading
> strbuf_trim):
>
> 	if (!get_sha1_hex(split[1]->buf, sha1) &&
> 	    !strcmp(split[1]->buf + 40, " ") {
> 		replace split[1] with "%s " abbrev
> 	}

Actually, we can do simpler: we still have the original line available, so if we don't find a sha1, we can just keep it. By just letting the few lines after the if enter the if, it just works:

		strbuf_trim(split[1]);
		if (!get_sha1(split[1]->buf, sha1)) {
			abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
			strbuf_reset(split[1]);
			strbuf_addf(split[1], "%s ", abbrev);
			strbuf_reset(line);
			for (i = 0; split[i]; i++)
				strbuf_addf(line, "%s", split[i]->buf);
		}

Show 9 quoted lines

>         while (!strbuf_getline(&line, f, '\n')) {
>                 if (line.len && line.len[0] == comment_line_char)
>                         continue;
>                 strbuf_rtrim(&line);
>                 if (!line.len)
>                         continue;
>                 abbrev_sha1_in_line(&line);
>                 string_list_append(lines, line.buf);
>         }

I took this (modulo s/line.len[0]/line.buf[0]/, and s/rtrim/trim/ to be robust to leading whitespace (not really important, but doesn't harm).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/