Re: [PATCH 14/14] Build in merge

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 1, 2008, 07:27 UTC
Message-ID: <7vzlp2oyh7.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <5e65b37998d1fdd9d314e48cea2cf67fd73ba8cd.1214879690.git.vmiklos@frugalware.org>

Miklos Vajna <vmiklos@frugalware.org> writes:

Show 20 quoted lines

> +	/* See if remote matches <name>~<number>, or <name>^ */
> +	ptr = strrchr(remote, '^');
> +	if (ptr && ptr[1] == '\0') {
> +		for (len = 0, ptr = remote + strlen(remote);
> +				remote < ptr && ptr[-1] == '^';
> +				ptr--)
> +			len++;
> +	}
> +	else {
> +		ptr = strrchr(remote, '~');
> +		if (ptr && ptr[1] != '0' && isdigit(ptr[1])) {
> +			len = ptr-remote;
> +			ptr++;
> +			for (ptr++; *ptr; ptr++)
> +				if (!isdigit(*ptr)) {
> +					len = 0;
> +					break;
> +				}
> +		}
> +	}

I still have problems with the above.  I'd write it this way:

	int len, early;
	...
	/* See if remote matches <name>^^^.. or <name>~<number> */
	for (len = 0, ptr = remote + strlen(remote);
	     remote < ptr && ptr[-1] == '^';
	     ptr--)
		len++;
	if (len)
		early = 1;
	else {
		early = 0;
		ptr = strrchr(remote, '~');
		if (ptr) {
			int seen_nonzero = 0;

			len++; /* count ~ */
			while (*++ptr && isdigit(*ptr)) {
				seen_nonzero |= (*ptr != '0');
				len++;
			}
			if (*ptr)
				len = 0; /* not ...~<number> */
			else if (seen_nonzero)
				early = 1;
			else if (len == 1)
				early = 1; /* "name~" is "name~1"! */
		}
	}
	if (len) {
		struct strbuf truname = STRBUF_INIT;
		strbuf_addstr(&truname, "refs/heads/");
		strbuf_addstr(&truname, remote);
		strbuf_setlen(&truname, len+11);
		if (resolve_ref(truname.buf, buf_sha, 0, 0)) {
			strbuf_addf(msg,
				    "%s\t\tbranch '%s'%s of .\n",
				    sha1_to_hex(remote_head->sha1),
				    truname.buf,
				    (early ? " (early part)" : ""));
			return;
		}
	}

The first loop is obvious. If the tail end is ^, we set "len" and see if the remainder is a branch name (and if that is the case we are always talking about an early part of it of the branch).

Otherwise, we do want to say "early part" if "$name~<number>" is given, and another special case is "$name~" which is "$name~1" these days. As long as number is not zero we would want to say "early part". Otherwise we would want to say it is a branch itself, not its early part.

I'll queue the fixed-up result in 'pu', but I have to tend to other topics before I can actually publish. Together with the fix to "head_invalid" confusion I mentioned in another message squashed in to this commit, all the tests now finally seem to pass on the topic branch.

Oh, by the way, you sent this and the previous round without marking them as RFC nor WIP, even though they obviously did not even pass the test suite. For example, without the head_invalid fix, anything that runs merge on detached head, most notably "git rebase -i", would not work at all.