Hi, first: Thank you very much for the work, I really owe you one!
On Sat, 2021-02-13 at 17:58 +0100, Erik Auerswald wrote: > 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. Hm, I'm not sure whether I understand this special case. When we have a tab as column separator, doesn't this imply that the second column is starting on a position n*8, (effectively equivalent to the first column), thus guaranteeing that the alignment is honored? So, if my brain isn't completely off the track, with -s'\t' there shouldn't be a difference whether we do -e -i or not, thus there needs not to be a special case. That being said, I don't see this exact distinction reflected in the code, so perhaps I just misunderstood. > > 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. I can only second this, with the patch my rather obscure (and complex) use case of printing thousands of lines of code works properly now! ~leo