Re: [PATCH 8/9] show, log: provide a --remerge-diff capability

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 21, 2021, 21:23 UTC
Message-ID: <211221.864k71r6kz.gmgdl@evledraar.gmail.com>
In-Reply-To: <5d5846be0bd67b8fd850ef2583beef064481a67d.1640109948.git.gitgitgadget@gmail.com>

On Tue, Dec 21 2021, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>

> +	if (rev->remerge_diff) {
> +		rev->remerge_objdir = tmp_objdir_create("remerge-diff");
> +		if (!rev->remerge_objdir)
> +			die(_("unable to create temporary object directory"));

It looks like the tmp_objdir_create() API is rather bad about mixing errors that would come with an errno with others, but shouldn't this be die_errno() in the case where it would fail due to a syscall? Even better would be passing a "gentle" to it and have it emit the appropriate errors.

> +	if (rev.remerge_diff)
> +		die(_("--remerge_diff does not make sense"));

s/_/-/

> +	struct merge_options o;
> +	struct commit_list *bases;
> +	struct merge_result res;

nit: could use "= { 0 }" instead of memset below.

> +	/* Re-merge the parents */
> +	merge_incore_recursive(&o,
> +			       bases, parents->item, parents->next->item,
> +			       &res);

style: odd not to have arguments that fit on the line on the line, i.e. "&o, bases, ...".

> +	/* Clean up the temporary object directory */
> +	if (opt->remerge_objdir != NULL)

style: if (!x) not if (x != NULL)