On Sun, Aug 06, 2023 at 12:31:57PM +1000, Mark Jamsek 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.

Thanks Mark! Ok by me for both diffs.

> ^There is an alternative fix for the diffstat duplication, which I've
> appended. I'm happy to go with whichever you like but do prefer the
> first because it is clearer; even though the second is shorter, it is
> not clear why we are doing it so would like to add a comment if you want
> to go with this fix.

I agree that the first fix reads better.