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

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

Hi,

On 13.02.21 15:17, Erik Auerswald wrote:
> 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.

It seems to me as if "untabify_input = true;" should be re-introduced
in one additional place to fix the regression from commit 553d347,
please see the attached patch version 3.

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

The latest version, i.e., v3 for now.

Thanks,
Erik
diff --git a/src/pr.c b/src/pr.c
index 22d032ba3..e193d632c 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;
@@ -1235,6 +1238,8 @@ init_parameters (int number_of_files)
          alignment */
       else if (!join_lines && col_sep_length == 1 && *col_sep_string == '\t')
         col_sep_string = column_separator;
+      else
+        untabify_input = true;
 
       truncate_lines = true;
       tabify_output = 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};