Re: Noah Misch 2017-04-07 <20170407021431.gb2658...@tornado.leadboat.com>
> > > I personally, and I know of a bunch of other regular contributors, find
> > > context diffs very hard to read.  Besides general dislike, for things
> > > like regression test output context diffs are just not well suited.
> > 
> > Personally, I disagree completely.  Unified diffs are utterly unreadable
> > for anything beyond trivial cases of small well-separated changes.
> > 
> > It's possible that regression failure diffs will usually fall into that
> > category, but I'm not convinced.
> 
> For reading patches, I frequently use both formats.  Overall, I perhaps read
> unified 3/4 of the time and context 1/4 of the time.
> 
> For regression diffs, I use PG_REGRESS_DIFF_OPTS=-u and have never converted a
> regression diff to context form.  Hence, +1 for the proposed change.

I've just had another case of horrible context diff from pg_regress.
I'd claim that regression diffs are particularly bad for context diffs
because one error will often trigger a whole chain of failures, which
will essentially make the whole file appear twice in the output,
whereas unified diffs would just put the original output and the error
right next to each other at the top. Having to scroll through a whole
.out file just to find "create extension; file not found" is very
inefficient.

It's nice that PG_REGRESS_DIFF_OPTS exists, but the diffs are often
coming from automated build logs where setting the variable after the
fact doesn't help.

Please consider the attached patch, extension packagers will thank
you.

Christoph
>From a9b1ef089bbcc36dc65e4acff60ae3b83ecd06e3 Mon Sep 17 00:00:00 2001
From: Christoph Berg <christoph.b...@credativ.de>
Date: Thu, 22 Nov 2018 13:33:42 +0100
Subject: [PATCH] Switch pg_regress to output unified diffs by default

---
 doc/src/sgml/regress.sgml     | 2 +-
 src/test/regress/pg_regress.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 673a8c2164..a33e2482f2 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -410,7 +410,7 @@ make standbycheck
     If you don't
     like the <command>diff</command> options that are used by default, set the
     environment variable <envar>PG_REGRESS_DIFF_OPTS</envar>, for
-    instance <literal>PG_REGRESS_DIFF_OPTS='-u'</literal>.  (Or you
+    instance <literal>PG_REGRESS_DIFF_OPTS='-C'</literal>.  (Or you
     can run <command>diff</command> yourself, if you prefer.)
    </para>
 
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 3248603da1..bb06624936 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -62,10 +62,10 @@ static char *shellprog = SHELLPROG;
  */
 #ifndef WIN32
 const char *basic_diff_opts = "";
-const char *pretty_diff_opts = "-C3";
+const char *pretty_diff_opts = "-U3";
 #else
 const char *basic_diff_opts = "-w";
-const char *pretty_diff_opts = "-w -C3";
+const char *pretty_diff_opts = "-w -U3";
 #endif
 
 /* options settable from command line */
-- 
2.19.1

Reply via email to