Stefan Sperling <stsp@stsp.name> wrote:
> I have another suggestion:
> 
> > [...]
> 
> The test doesn't check whether 3-way merge or search/replace was used.
> Which kind of defeats the point, doesn't it?
> 
> 'got patch' 3-way merge has the same possible outcomes as 'got cherrypick',
> Except merge is only used if the file already exists on disk, and add/delete
> is done elsewhere, so we are left with only the following two status codes
> as they appear in 'got cherrypick':
> 
> 	       G      file was merged
> 	       C      file was merged and conflicts occurred during merge
> 
> I would propose to make 'got patch' use status code as follows:
> 
> 	       M      file was modified
> 	       G      file was merged using a merge-base found in repository
> 	       C      file was merged and conflicts occurred during merge
> 	       D      file was deleted
> 	       A      file was added
> 	       #      failed to patch the file
> 
> And then our tests could check for M or G/C to see which algorithm
> was used to apply changes to a file.  What do you think?

it's awesome.

The thought that we weren't correctly communicating to the user wheter
we did the 3-way merge or the dumb patch occurred to me, but I wasn't
sure how to deal with it.  I thought of doing something with a callback,
but i didn't really liked the idea.

Instead, using G instead of M as status code is just the only correct
thing to do.  It's already used in other places and doesn't add noise to
the output.  It's so obvious that I'm ashamed that I haven't thought
about it before.

This also reminded me that I'm not documenting the 'C' status in the
manual!

Here's a diff (built on top of the previous one) that uses the G status
code when merging, improves the tests to explicitly look for it and
updates the manpage blatantly copying your suggestion :)

Thanks a lot!

diff 814afc913552a78a1df6d849a3f916e90ce4ad65 /home/op/w/got
blob - 9d65d8aa6b3211dfb9e231199f0bc3e522f4dfa8
file + got/got.1
--- got/got.1
+++ got/got.1
@@ -1333,6 +1333,8 @@ contains multiple patches, then attempt to apply each 
 Show the status of each affected file, using the following status codes:
 .Bl -column XYZ description
 .It M Ta file was modified
+.It G Ta file was merged using a merge-base found in the repository
+.It C Ta file was merged and conflicts occurred during merge
 .It D Ta file was deleted
 .It A Ta file was added
 .It # Ta failed to patch the file
blob - 1fc82c6604c60d1e3961f7d6670b26b781db97cc
file + lib/patch.c
--- lib/patch.c
+++ lib/patch.c
@@ -777,6 +777,8 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 			unlink(newpath);
 	} else if (*overlapcnt != 0)
 		err = report_progress(pa, old, new, GOT_STATUS_CONFLICT, NULL);
+	else if (do_merge)
+		err = report_progress(pa, old, new, GOT_STATUS_MERGE, NULL);
 	else
 		err = report_progress(pa, old, new, GOT_STATUS_MODIFY, NULL);
 
blob - 5d79b58c66cb3e7160cfdb8ce13e13901219e70e
file + regress/cmdline/patch.sh
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -1482,13 +1482,22 @@ test_patch_merge_simple() {
 	fi
 
 	(cd $testroot/wt && got patch $testroot/old.diff) \
-		2>&1 > /dev/null
+		> $testroot/stdout
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		test_done $testroot $ret
 		return 1
 	fi
 
+	echo 'G  numbers' > $testroot/stdout.expected
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout $testroot/stdout.expected
+		test_done $testroot $ret
+		return 1
+	fi
+
 	jot 10 | sed -e s/4/four/ -e s/6/six/ > $testroot/wt/numbers.expected
 	cmp -s $testroot/wt/numbers $testroot/wt/numbers.expected
 	ret=$?
@@ -1539,7 +1548,7 @@ test_patch_merge_conflict() {
 	fi
 
 	(cd $testroot/wt && got patch $testroot/old.diff) \
-		>/dev/null 2>&1
+		> $testroot/stdout 2>/dev/null
 	ret=$?
 	if [ $ret -eq 0 ]; then
 		echo "got patch merged a diff that should conflict" >&2
@@ -1547,6 +1556,15 @@ test_patch_merge_conflict() {
 		return 1
 	fi
 
+	echo 'C  numbers' > $testroot/stdout.expected
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout $testroot/stdout.expected
+		test_done $testroot $ret
+		return 1
+	fi
+
 	# XXX: prefixing every line with a tab otherwise got thinks
 	# the file has conflicts in it.
 	cat <<-EOF > $testroot/wt/numbers.expected