Subject: bug#46422: 'pr' screws up tabstops in multicolumn outpt?

From: Erik Auerswald <[email-masked]>
Date: Thu, 11 Feb 2021 20:20:05 +0100

Hi,

On Thu, Feb 11, 2021 at 06:09:28PM +0100, Leonard Janis Robert König wrote:
> On Thu, 2021-02-11 at 16:45 +0100, Erik Auerswald wrote:
> > On Thu, Feb 11, 2021 at 04:12:54PM +0100, Leonard Janis Robert
> > König wrote:
> > > On Thu, 2021-02-11 at 13:00 +0100, Erik Auerswald wrote:
> > > > On Wed, Feb 10, 2021 at 01:42:29PM +0100, Leonard Janis Robert
> > > > König wrote:
> > > > > I'm sorry if I this is not a bug but to be expected, but I thnk
> > > > > pr doesn't get the alignment of tabs in multicolumn output
> > > > > right.  [...]  This seems *kind* of related to multi-column
> > > > > merged output, as was discussed some years ago here:
> > > > > https://lists.gnu.org/archive/html/bug-coreutils/2007-03/msg00121.html
> > > > 
> > > > This thread contains the bug-introducing patch in message
> > > > https://lists.gnu.org/archive/html/bug-coreutils/2007-03/msg00160.html
> > > > 
> > > > This is commit 553d347d3e08e00ee4f9df520b37c964c3f26e28.
> > > 
> > > ah, thanks for digging, I read the message but must have missed
> > > the patch.
> > > 
> > > > That commit removed the 'assume -e' part of the POSIX description
> > > > of the -COLUMN option from GNU pr.
> > > [...]
> > Your test case requires expanding tabs during input, which is
> > the reason that "expand | pr" could be used as a workaround (with
> > "expand | pr | unexpand", pr would not need to mess with tabs at all,
> > but I do think that GNU pr is currently buggy and should be fixed).
> 
> Absolutely, expand would be a workaround (I happen to use `pr -e | pr`
> in my script, for other reasons).
> 
> I've looked a bit further through the code but there's hardly a single
> place that needs to be touched in order to not introduce other bugs
> again.  For now I can only put it on my to-do list to fix, but no idea
> when I get around doing it.

I have found a fix to the problem described by you.  I am quite sure that
this is not *correct*, but I did not find a way to make print_sep_string()
account for tabs that did not break quite a few existing tests, even if
the merged files problem from 2007 and this columnating bug were both
fixed.  Thus I just tighten the 2007 bug fix to apply in less cases.
This way all existing tests pass, and a new one pertaining to this bug
report passes, too.  I do think this is in the same spirit as the "fix"
from 2007 (commit 553d347d3e08e00ee4f9df520b37c964c3f26e28).

See the following inline patch:

--------8<--------
diff --git a/src/pr.c b/src/pr.c
index 22d032ba3..ad1e36769 100644
--- a/src/pr.c
+++ b/src/pr.c
@@ -1237,6 +1237,8 @@ init_parameters (int number_of_files)
         col_sep_string = column_separator;
 
       truncate_lines = true;
+      if (!parallel_files)
+        untabify_input = true;
       tabify_output = true;
     }
   else
diff --git a/tests/pr/pr-tests.pl b/tests/pr/pr-tests.pl
index b7d868cf8..0894d3804 100755
--- a/tests/pr/pr-tests.pl
+++ b/tests/pr/pr-tests.pl
@@ -474,6 +474,12 @@ push @Tests,
     {IN=>{2=>"a\n"}},
      {OUT=>"a\t\t\t\t  \t\t\ta\n"} ];
 
+# Exercise a bug with pr -t -2 (bug #46422)
+push @Tests,
+   ['mcol-w-tabs', '-t -2',
+    {IN=>"x\tx\tx\tx\tx\nx\tx\tx\tx\tx\n"},
+     {OUT=>"x\tx\tx\tx\tx   x\t    x\t    x\t    x\t    x\n"} ];
+
 @Tests = triple_test \@Tests;
 
 my $save_temps = $ENV{DEBUG};
-------->8--------

It is up to the GNU Coreutils maintainers if they want to add this
additional band-aid to the interesting 'pr' code or not.  Adherents to
test-driven development would probably like this approach.  ;-)

Thanks,
Erik
-- 
Bugs are like mushrooms - found one, look around for more...
                        -- Al Viro