From: Omar Polo <op@omarpolo.com>
Subject: Re: allow log -x to accept keywords and fix diffstat duplicates
To: Mark Jamsek <mark@jamsek.com>
Cc: gameoftrees@openbsd.org
Date: Mon, 07 Aug 2023 08:48:45 +0200

On 2023/08/06 12:31:57 +1000, Mark Jamsek <mark@jamsek.com> wrote:
> I'm inlining two somewhat related diffs because they're both small and
> also both suggested/reported by Lucas on IRC. Neither have tests yet for
> different reasons but I wanted to send these now.
> 
> The first fixes^ the diffstat duplication when all three of -dPp are
> used (e.g., got log -dPp). I actually remember seeing this when first
> implementing diffstat and could've sworn I already had this check
> guarding the get_changed_paths() calls but must either be misremembering
> or ended up committing the wrong diff--I'd say the former is more
> likely :)
> 
> The fix is simple enough, we only need to get changed paths once as
> they are used for the diffstat (-d) and path report (-P), so only call
> it once when both options are specified; we already had the similar
> -dp path guarded.
> 
> Also, I've not added regress for this because I think op has already
> written a test but if not, I'll write a test before committing.
> 
> The second allows 'got log -x' to take keywords :)
> Please see the docs for this; I opted for brevity as just above the -x
> docs we explain the commit argument in detail under -c. However, perhaps
> it might be better to provide a slightly less abridged version? I'll
> send a test for this later today but I've got to step out for now.

late to the party, but ok op@ too.  I prefer the first diff too.

I haven't written a full-fledged test, just what I'm attaching below.
Feel free to include it when you commit the fix, or scratch it and
rewrite something better.

thanks!

diff /home/op/w/got
commit - 0f2e686eec562e28977521d25101acfa4396b47a
path + /home/op/w/got
blob - e2f69ad6bfabb6d873c7f6e79c7277eb9552ff14
file + regress/cmdline/log.sh
--- regress/cmdline/log.sh
+++ regress/cmdline/log.sh
@@ -896,7 +896,19 @@ EOF
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
 	fi
+
+	# try again but with -Pp also
+	(cd $testroot/wt && got log -dPp | grep -A2 '^ [MDmA]' | sed '/^--/d' > \
+	    $testroot/stdout)
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
 	test_done "$testroot" "$ret"
 }