Subject: [PATCH] Re: bug#46422: 'pr' screws up tabstops in multicolumn outpt?

From: Erik Auerswald <[email-masked]>
Date: Sat, 13 Feb 2021 15:17:08 +0100

On 11.02.21 20:20, Erik Auerswald wrote:
> 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 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).

I think the attached patch is a better fix than my previous one,
because it applies the special treatment of TAB as separator more
consistently.  It may still not be complete (the code seems quite
convoluted to me) but I do think it improves the situation
significantly, and does not make it worse.

The code does not try to create equal width columns when using a
TAB as column separator.  This is made clear through comments:

1018 /* Tabification is assumed for multiple columns. */
...
1031     /* It's rather pointless to define a TAB separator with column
1032        alignment */

Thus the intent of the code seems to be follow the general idea
of using equal width columns by "assuming Tabification," i.e.,
working as if the options -e and -i were given, as specified by
POSIX, unless the column separator has been changed to a TAB.
The attached patch results in following through with this in more
cases, fixing this bug (bug#46422) without introducing known
regressions.

The patch adds more test cases.  One identical to the new test
from my previous patch, another generalizes the case from 2007
to use '-2 -s' to trigger special treatment with TAB as separator.

Creating three column output as done in the bug report from 2007
automatically aligns the columns with the default TAB stops of pr,
thus the patch adds another variant of the 2007 case merging two
files.  Merging files (-m) is done with a slightly different code
path from -NUMBER, while both create columns.

I'd like to ask the GNU Coreutils maintainers to consider merging
the attached patch.

Thanks,
Erik
diff --git a/src/pr.c b/src/pr.c
index 22d032ba3..d518b81ab 100644
--- a/src/pr.c
+++ b/src/pr.c
@@ -1226,7 +1226,10 @@ init_parameters (int number_of_files)
           if (join_lines)
             col_sep_string = line_separator;
           else
-            col_sep_string = column_separator;
+            {
+              col_sep_string = column_separator;
+              untabify_input = true;
+            }
 
           col_sep_length = 1;
           use_col_separator = true;
diff --git a/tests/pr/pr-tests.pl b/tests/pr/pr-tests.pl
index b7d868cf8..ed5c61dc9 100755
--- a/tests/pr/pr-tests.pl
+++ b/tests/pr/pr-tests.pl
@@ -467,6 +467,14 @@ push @Tests,
     {IN=>{3=>"x\ty\tz\n"}},
      {OUT=>join("\t", qw(a b c m n o x y z)) . "\n"} ];
 
+# Exercise a variant of the bug with pr -m -s (commit 553d347)
+# test 2 files, too (merging 3 files automatically aligns columns on TAB stops)
+push @Tests,
+   ['merge-2-w-tabs', '-m -s -t',
+    {IN=>{1=>"a\tb\tc\n"}},
+    {IN=>{2=>"m\tn\to\n"}},
+     {OUT=>join("\t", qw(a b c m n o)) . "\n"} ];
+
 # This resulted in reading invalid memory before coreutils-8.26
 push @Tests,
    ['asan1', "-m -S'\t\t\t' -t",
@@ -474,6 +482,18 @@ 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"} ];
+
+# generalize case from commit 553d347 (problem results from -s, not -m)
+push @Tests,
+   ['mcol-w-tabs-w-tabsep', '-t -2 -s',
+    {IN=>"x\tx\tx\tx\tx\nx\tx\tx\tx\tx\n"},
+     {OUT=>"x\tx\tx\tx\tx\tx\tx\tx\tx\tx\n"} ];
+
 @Tests = triple_test \@Tests;
 
 my $save_temps = $ENV{DEBUG};