Download raw body.
allow log -x to accept keywords and fix diffstat duplicates
Omar Polo <op@omarpolo.com> wrote:
> 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!
Thanks, op! Sorry I didn't get this out last night.
I've just added the other combinations to your diff so we test for -dPp,
-dP, and -dp, and also added some 'log -x keyword' cases to the keywords
regress.
The diff, however, also includes quite a few log regress copypasta
errors where I cmp(1)'d stdout{,.expected} instead of stderr! I was
very fortunate that we are producing the correct output; the tests and
behaviour are all correct but I failed to check that it indeed was
right. There was also one diff keyword case with the same problem
although the test still wrongly checked for the original BASE output,
which I have also now fixed.
I checked the many keyword tests I did around that time and it looks
like I only committed this error repeatedly in the log tests and that
last diff keyword test. I'm going to double check them again tomorrow
with fresher eyes to be sure though.
I will _never_ copypasta regress scripts ever again; that was equal
parts stupid and lucky!
btw, regarding log -dP, I was wondering if we should even make these
mutually exclusive? It seems like it might be the right thing to do
because it doesn't really make sense to ask for both as they both print
the changed paths, just the diffstat annotates them with lines added
and/or removed plus a summary total. It's fine as it is now the
duplication is fixed but I was thinking about this when writing the
test so thought I'd float the idea.
-----------------------------------------------
commit c9aea17ec1e90231f5beb84aaf51398dc915d705 (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Mon Aug 7 13:26:37 2023 UTC
got: regress for log -x keyword and log -dPp combinations
Plus multiple copypasta fixes where I was cmp(1)ing stdout instead of
stderr! Fortunately, we were doing the right thing despite failing to
check it properly. The log -dPp test is from op@.
M regress/cmdline/diff.sh | 3+ 3-
M regress/cmdline/log.sh | 80+ 14-
2 files changed, 83 insertions(+), 17 deletions(-)
diff 07d8d6083dc8bd84b2c7ca96068ce3c35f8c1dbf c9aea17ec1e90231f5beb84aaf51398dc915d705
commit - 07d8d6083dc8bd84b2c7ca96068ce3c35f8c1dbf
commit + c9aea17ec1e90231f5beb84aaf51398dc915d705
blob - 40d362b35f6816f824c7f4f341e1ec65900ed41e
blob + f24be2ec6def2b1c4206a2f88553c21f47ae2eb9
--- regress/cmdline/diff.sh
+++ regress/cmdline/diff.sh
@@ -2254,14 +2254,14 @@ test_diff_commit_keywords() {
return 1
fi
- echo "'-c BASE' requires work tree" > "$testroot/stderr.expected"
+ echo "got: '-c :base' requires work tree" > "$testroot/stderr.expected"
got diff -r "$testroot/repo" -c:base -c:head 2> $testroot/stderr
- cmp -s $testroot/stdout.expected $testroot/stdout
+ cmp -s $testroot/stderr.expected $testroot/stderr
ret=$?
if [ $ret -ne 0 ]; then
- diff -u $testroot/stdout.expected $testroot/stdout
+ diff -u $testroot/stderr.expected $testroot/stderr
fi
test_done "$testroot" "$ret"
blob - e2f69ad6bfabb6d873c7f6e79c7277eb9552ff14
blob + 5510011eead2b8ebb0a825d375af14b202452692
--- regress/cmdline/log.sh
+++ regress/cmdline/log.sh
@@ -896,7 +896,43 @@ EOF
ret=$?
if [ $ret -ne 0 ]; then
diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
fi
+
+ # try again with -dPp
+ (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
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # try again with -dP
+ (cd $testroot/wt && got log -dP | 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
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # try again with -dp
+ (cd $testroot/wt && got log -dp | 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"
}
@@ -944,6 +980,17 @@ test_log_commit_keywords() {
return 1
fi
+ # request same set of commits now with log -x
+ got log -r "$testroot/repo" -scmaster:- -xmaster:-15 > $testroot/stdout
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
(cd $testroot/wt && got update -c:head:-8 > /dev/null)
ret=$?
if [ $ret -ne 0 ]; then
@@ -972,6 +1019,25 @@ test_log_commit_keywords() {
return 1
fi
+ # from head to the base commit using -x
+ printf '%s %-7s commit number 16\n' "$d" "master" > \
+ $testroot/stdout.expected
+ for i in $(seq 16 9); do
+ printf '%s %.7s commit number %s\n' \
+ "$d" $(pop_idx $i $@) "$(( i-1 ))" \
+ >> $testroot/stdout.expected
+ done
+
+ (cd $testroot/wt && got log -sc:head -x:base > $testroot/stdout)
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
# if + modifier is too great, use HEAD commit
printf '%s %-7s commit number %s\n' "$d" master 16 > \
$testroot/stdout.expected
@@ -1051,10 +1117,10 @@ test_log_commit_keywords() {
(cd $testroot/wt && got log -c::base:+ 2> $testroot/stderr)
- cmp -s $testroot/stdout.expected $testroot/stdout
+ cmp -s $testroot/stderr.expected $testroot/stderr
ret=$?
if [ $ret -ne 0 ]; then
- diff -u $testroot/stdout.expected $testroot/stdout
+ diff -u $testroot/stderr.expected $testroot/stderr
test_done "$testroot" "$ret"
return 1
fi
@@ -1064,10 +1130,10 @@ test_log_commit_keywords() {
(cd $testroot/wt && got log -c:head:-: 2> $testroot/stderr)
- cmp -s $testroot/stdout.expected $testroot/stdout
+ cmp -s $testroot/stderr.expected $testroot/stderr
ret=$?
if [ $ret -ne 0 ]; then
- diff -u $testroot/stdout.expected $testroot/stdout
+ diff -u $testroot/stderr.expected $testroot/stderr
test_done "$testroot" "$ret"
return 1
fi
@@ -1077,10 +1143,10 @@ test_log_commit_keywords() {
(cd $testroot/wt && got log -cmaster::+ 2> $testroot/stderr)
- cmp -s $testroot/stdout.expected $testroot/stdout
+ cmp -s $testroot/stderr.expected $testroot/stderr
ret=$?
if [ $ret -ne 0 ]; then
- diff -u $testroot/stdout.expected $testroot/stdout
+ diff -u $testroot/stderr.expected $testroot/stderr
test_done "$testroot" "$ret"
return 1
fi
@@ -1090,10 +1156,10 @@ test_log_commit_keywords() {
(cd $testroot/wt && got log -cmaster:1+ 2> $testroot/stderr)
- cmp -s $testroot/stdout.expected $testroot/stdout
+ cmp -s $testroot/stderr.expected $testroot/stderr
ret=$?
if [ $ret -ne 0 ]; then
- diff -u $testroot/stdout.expected $testroot/stdout
+ diff -u $testroot/stderr.expected $testroot/stderr
test_done "$testroot" "$ret"
return 1
fi
@@ -1103,10 +1169,10 @@ test_log_commit_keywords() {
(cd $testroot/wt && got log -c:base:-1:base:-1 2> $testroot/stderr)
- cmp -s $testroot/stdout.expected $testroot/stdout
+ cmp -s $testroot/stderr.expected $testroot/stderr
ret=$?
if [ $ret -ne 0 ]; then
- diff -u $testroot/stdout.expected $testroot/stdout
+ diff -u $testroot/stderr.expected $testroot/stderr
test_done "$testroot" "$ret"
return 1
fi
@@ -1116,10 +1182,10 @@ test_log_commit_keywords() {
(cd $testroot/wt && got log -cmain:-main:- 2> $testroot/stderr)
- cmp -s $testroot/stdout.expected $testroot/stdout
+ cmp -s $testroot/stderr.expected $testroot/stderr
ret=$?
if [ $ret -ne 0 ]; then
- diff -u $testroot/stdout.expected $testroot/stdout
+ diff -u $testroot/stderr.expected $testroot/stderr
test_done "$testroot" "$ret"
return 1
fi
@@ -1129,10 +1195,10 @@ test_log_commit_keywords() {
(cd $testroot/wt && got log -c:base:*1 2> $testroot/stderr)
- cmp -s $testroot/stdout.expected $testroot/stdout
+ cmp -s $testroot/stderr.expected $testroot/stderr
ret=$?
if [ $ret -ne 0 ]; then
- diff -u $testroot/stdout.expected $testroot/stdout
+ diff -u $testroot/stderr.expected $testroot/stderr
test_done "$testroot" "$ret"
return 1
fi
--
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
allow log -x to accept keywords and fix diffstat duplicates