Re: 'pr' expands tabs incorrectly?

[Jim Meyering]

"Macomson, Wis" <address@hidden> wrote:
> I don't know, but line 1268 in "pr.c" seem to peremptorily set
> "untabify_input", which could definitely get the spaces in there.
>
>   if (columns > 1)
>     {
>       . . .
>       truncate_lines = true;
>       untabify_input = true;

It just so happens that removing that line is the solution.
I stepped through a couple of test cases, and found nothing in the
code, documentation, or test suite to contradict that conclusion.
I've included the patch below.

> This also raises a question about the preceding line "truncate_lines =
> true;" which would could prevent me from freely concatenating long lines
> from multiple files in a merge.

You can use the -J option to turn off the default truncation.
-m is documented to truncate.  The "info" documentation is the
authoritative source, not the code :-)
However, if you're convinced that some comment is false,
I'd really appreciate a patch correcting it.

> This appears to conflict with "-s",
> which is described under the "-w" option (pr.c, line 282) as:

>
>     ...-s[CHAR] turns off the default page width and any line
> truncation. Lines of full
>         length will be merged, regardless of the column options set. ...
>
> This has not been tested, though.

pr.c is one of the hairiest chunks of code in the coreutils package,
and its test suite is impressive, but I have no illusions on this front.
There is so much global state in the implementation, and there are so
many options, that I'm confident that there are other bugs lurking there.
>From a design standpoint, pr.c is by far the ugliest program in the
coreutils.

====================================================================

        Fix a bug in how pr -m -s works.
        * NEWS: Describe how the fix affects pr.
        * src/pr.c (init_parameters): The --merge (-m) option does
        not imply --expand-tabs (-e), so don't set "untabify_input".
        Reported by Wis Macomson.
        * tests/misc/pr: New file.  Test for the above fix.
        * tests/misc/Makefile.am (TESTS): Add pr.
        * THANKS: Update.

diff --git a/NEWS b/NEWS
index 57c51e1..4efb18f 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,10 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   the DF_BLOCK_SIZE, BLOCK_SIZE, and BLOCKSIZE environment variables.  It
   is still affected by POSIXLY_CORRECT, though.

+  Using pr -m -s (i.e. merging files, with TAB as the output separator)
+  no longer inserts extraneous spaces between output columns.
+
+
 * Noteworthy changes in release 6.8 (2007-02-24) [not-unstable]

 ** Bug fixes
diff --git a/THANKS b/THANKS
index 1d7aae3..ad05b13 100644
--- a/THANKS
+++ b/THANKS
@@ -519,6 +519,7 @@ William Bader                       address@hidden
 William Dowling                     address@hidden
 William Lewis                       address@hidden
 wiregauze                           address@hidden
+Wis Macomson                        address@hidden
 Wojciech Purczynski                 address@hidden
 Wolfram Kleff                       address@hidden
 Won-kyu Park                        address@hidden
diff --git a/src/pr.c b/src/pr.c
index bd694f4..e0aea22 100644
--- a/src/pr.c
+++ b/src/pr.c
@@ -1,5 +1,5 @@
 /* pr -- convert text files for printing.
-   Copyright (C) 88, 91, 1995-2006 Free Software Foundation, Inc.
+   Copyright (C) 88, 91, 1995-2007 Free Software Foundation, Inc.

    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -1265,7 +1265,6 @@ init_parameters (int number_of_files)
         col_sep_string = column_separator;

       truncate_lines = true;
-      untabify_input = true;
       tabify_output = true;
     }
   else
diff --git a/tests/misc/Makefile.am b/tests/misc/Makefile.am
index 1aa3516..4d6a754 100644
--- a/tests/misc/Makefile.am
+++ b/tests/misc/Makefile.am
@@ -40,6 +40,7 @@ TESTS_ENVIRONMENT = \
 # will execute the test script rather than the standard utility.

 TESTS = \
+  pr \
   df-P \
   pwd-unreadable-parent \
   cut \
diff --git a/tests/misc/pr b/tests/misc/pr
new file mode 100755
index 0000000..a40ea31
--- /dev/null
+++ b/tests/misc/pr
@@ -0,0 +1,61 @@
+#!/bin/sh
+# -*- perl -*-
+# Exercise a bug with pr -m -s
+
+# Copyright (C) 2007 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+: ${PERL=perl}
+: ${srcdir=.}
+
+. $srcdir/../envvar-check
+
+$PERL -e 1 > /dev/null 2>&1 || {
+  echo 1>&2 "$0: configure didn't find a usable version of Perl," \
+    "so can't run this test"
+  exit 77
+}
+
+exec $PERL -w -I$srcdir/.. -MCoreutils -- - <<\EOF
+#/
+require 5.003;
+use strict;
+
+(my $program_name = $0) =~ s|.*/||;
+
+$ENV{PROG} = 'pr';
+my $ME = $ENV{PROG};
+
+# Turn off localization of executable's ouput.
address@hidden(LANGUAGE LANG LC_ALL)} = ('C') x 3;
+
+my @Tests =
+  (
+   ['merge-w-tabs', '-m -s -t',
+    {IN=>{1=>"a\tb\tc\n"}},
+    {IN=>{2=>"m\tn\to\n"}},
+    {IN=>{3=>"x\ty\tz\n"}},
+     {OUT=>join("\t", qw(a b c m n o x y z)) . "\n"} ],
+  );
+
+my $save_temps = $ENV{DEBUG};
+my $verbose = $ENV{VERBOSE};
+
+my $prog = $ENV{PROG} || die "$0: \$PROG not specified in environment\n";
+my $fail = run_tests ($program_name, $prog, address@hidden, $save_temps, 
$verbose);
+exit $fail;
+EOF
M ChangeLog
M NEWS
M THANKS
M src/pr.c
M tests/misc/Makefile.am
A tests/misc/pr
Committed as 553d347d3e08e00ee4f9df520b37c964c3f26e28