bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Erik Auerswald
Subject: bug#46422: [PATCH] Re: bug#46422: 'pr' screws up tabstops in multicolumn outpt?
Date: Sat, 13 Feb 2021 15:17:08 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0

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

Attachment: coreutils-pr-fix_bug_46422.v2.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]