reflog doesn't include "(merge)" for merge commits

[rcjsuen]

When you use the command line, merge commits will have commit (merge): recorded in the reflog as a reminder that a merge commit was created. libgit2 doesn't do this though. It only handles the simple commit: case and the first commit (initial): case.

Reproduction steps

1. Create a merge commit using libgit2's APIs.
2. Check the output of git reflog.

Expected behavior

cf97c9d HEAD@{0}: commit (merge): Merge branch 'other'

Actual behavior

d50f21e HEAD@{0}: commit: Merge branch 'other'

Version of libgit2 (release number or SHA1)

Looks like the code is still the same on 8d3b39a.

libgit2/src/refs.c

Lines 1148 to 1153 in 8d3b39a

	if ((error = git_commit_lookup(&commit, repo, id)) < 0 ||
	(error = git_buf_printf(&reflog_msg, "%s%s: %s",
	operation ? operation : "commit",
	git_commit_parentcount(commit) == 0 ? " (initial)" : "",
	git_commit_summary(commit))) < 0)
	goto done;

Operating system(s) tested

Windows 10

[ethomson]

Thanks for the report @rcjsuen !

This seems like a straightforward fix, I'm going to add the "Easy Fix" and "Up for Grabs" tags in case someone wants to submit a pull request!

[ghost]

I was looking here at #4094 this week and was coming toward a similar conclusion but above fix is better than mine. I am was happy to learn I was in the right spot and looking to change the same field. I added above fix into v0.25.1 (2fcb870) and all tests passed on Windows 7.

I looked at adding a new test and found that reflog is not tested very much, at least in this fashion, for anything besides "commit (initial)" which failed only four times after breaking on purpose, 3 in rebase::merge and 1 in commit::write. What I think I learned about a merge commit from testing point of view is it crosses unit test boundaries since merge puts repo in a merge state and until the commit completes.

[richardipsum]

Shouldn't this issue have been automatically closed by github when #4143 got merged?

[ethomson]

I don't know how the automatic matching works, exactly, but I'll close it manually.