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

On Tue, Dec 21, 2021 at 1:28 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

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

quoted

From: Elijah Newren <redacted>

quoted

+     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.

I can switch to die_errno().

quoted

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

s/_/-/

Indeed, thanks.

quoted

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

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

Sure, I can make that change.

quoted

+     /* 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, ...".

Yes, but this groups all the ancestors so nicely as opposed to the
typical happenstance of whatever fits on a line.  ;-)

quoted

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

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

Ok, will change.