Re: [PATCH] git-mv: improve error message for conflicted file

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 18, 2020, 00:07 UTC
Message-ID: <xmqqeep9d6tm.fsf@gitster.c.googlers.com>
In-Reply-To: <pull.678.git.1595028293855.gitgitgadget@gmail.com>

"Chris Torek via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 14 quoted lines

> -		} else if (cache_name_pos(src, length) < 0)
> -			bad = _("not under version control");
> -		else if (lstat(dst, &st) == 0 &&
> +		} else if (cache_name_pos(src, length) < 0) {
> +			/*
> +			 * This occurs for both untracked files *and*
> +			 * files that are in merge-conflict state, so
> +			 * let's distinguish between those two.
> +			 */
> +			struct cache_entry *ce = cache_file_exists(src, length, ignore_case);
> +			if (ce == NULL)
> +				bad = _("not under version control");
> +			else
> +				bad = _("must resolve merge conflict first");

The original did not care about the cache entry itself, and that is why cache_name_pos() was used. Now you care what cache entry is at that position, running both calls is quite wasteful.

Would it work better to declare "struct cache_entry *ce" in the scope that surrounds this if/elseif cascade and then rewrite this part more like so:

	} else if (!(ce = cache_file_exists(...)) {
		bad = _("not tracked");
	} else if (ce_stage(ce)) {
        	bad = _("conflicted");
	} else if (lstat(...)) { ...