GNU bug report logs - #31105
[PATCH] Restart and reset ANSI escapes on each line for compatibility with less -R

Previous Next

Package: diffutils;

Reported by: "Dennis Lambe Jr." <malsyned <at> malsyned.net>

Date: Mon, 9 Apr 2018 08:04:02 UTC

Severity: normal

Tags: patch

Done: Jim Meyering <jim <at> meyering.net>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31105 in the body.
You can then email your comments to 31105 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-diffutils <at> gnu.org:
bug#31105; Package diffutils. (Mon, 09 Apr 2018 08:04:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to "Dennis Lambe Jr." <malsyned <at> malsyned.net>:
New bug report received and forwarded. Copy sent to bug-diffutils <at> gnu.org. (Mon, 09 Apr 2018 08:04:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Dennis Lambe Jr." <malsyned <at> malsyned.net>
To: bug-diffutils <at> gnu.org
Subject: [PATCH] Restart and reset ANSI escapes on each line for
 compatibility with less -R
Date: Sun, 08 Apr 2018 19:59:31 -0400
[Message part 1 (text/plain, inline)]
Hello!

GNU less can display ANSI-colored text with the -R flag, but this  
support has some limitations. One of them is that if an escape  
sequence starts on one line and ends on a different line, only the  
first line will be colored in less.

As a result, when diff creates colored output with multi-line deletes  
or adds, less will only color the first line.

I've attached a patch to reset ANSI color to the default at the end of  
every line and restart it at the beginning of the next. It patches  
normal and context mode. Side-by-side already worked in my testing.

I hope it's useful to you. Please let me know if there are changes you  
would like made before you can accept it.

--Dennis Lambe Jr.
[99-color-escapes-every-line.patch (text/x-patch, attachment)]

Information forwarded to bug-diffutils <at> gnu.org:
bug#31105; Package diffutils. (Mon, 09 Apr 2018 19:01:02 GMT) Full text and rfc822 format available.

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Dennis Lambe Jr." <malsyned <at> malsyned.net>
To: bug-diffutils <at> gnu.org
Subject: Re: [bug-diffutils] bug#31105: [PATCH] Restart and reset ANSI
 escapes on each line for compatibility with less -R
Date: Mon, 09 Apr 2018 11:53:17 -0400
[Message part 1 (text/plain, inline)]
The previous patch had an error in the first hunk for normal.c. I  
forgot to remove an "if" line when I removed the following conditional  
line. Due to its placement before a "for" line that tested the same  
condition, it didn't affect the behavior of the patch, but it was  
unsightly. I've attached an updated patch.

Quoting "Dennis Lambe Jr." <malsyned <at> malsyned.net>:

> Hello!
>
> GNU less can display ANSI-colored text with the -R flag, but this   
> support has some limitations. One of them is that if an escape   
> sequence starts on one line and ends on a different line, only the   
> first line will be colored in less.
>
> As a result, when diff creates colored output with multi-line  
> deletes  or adds, less will only color the first line.
>
> I've attached a patch to reset ANSI color to the default at the end  
> of  every line and restart it at the beginning of the next. It  
> patches  normal and context mode. Side-by-side already worked in my  
> testing.
>
> I hope it's useful to you. Please let me know if there are changes  
> you  would like made before you can accept it.
>
> --Dennis Lambe Jr.
>

[99-color-escapes-every-line-v2.patch (text/x-patch, attachment)]

Information forwarded to bug-diffutils <at> gnu.org:
bug#31105; Package diffutils. (Sat, 29 Dec 2018 21:14:02 GMT) Full text and rfc822 format available.

Message #11 received at 31105 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: "Dennis Lambe Jr." <malsyned <at> malsyned.net>
Cc: 31105 <at> debbugs.gnu.org
Subject: Re: [bug-diffutils] bug#31105: bug#31105: [PATCH] Restart and reset
 ANSI escapes on each line for compatibility with less -R
Date: Sat, 29 Dec 2018 13:13:37 -0800
[Message part 1 (text/plain, inline)]
On Mon, Apr 9, 2018 at 12:02 PM Dennis Lambe Jr. <malsyned <at> malsyned.net> wrote:
>
> The previous patch had an error in the first hunk for normal.c. I
> forgot to remove an "if" line when I removed the following conditional
> line. Due to its placement before a "for" line that tested the same
> condition, it didn't affect the behavior of the patch, but it was
> unsightly. I've attached an updated patch.
>
> Quoting "Dennis Lambe Jr." <malsyned <at> malsyned.net>:
>
> > Hello!
> >
> > GNU less can display ANSI-colored text with the -R flag, but this
> > support has some limitations. One of them is that if an escape
> > sequence starts on one line and ends on a different line, only the
> > first line will be colored in less.
> >
> > As a result, when diff creates colored output with multi-line
> > deletes  or adds, less will only color the first line.
> >
> > I've attached a patch to reset ANSI color to the default at the end
> > of  every line and restart it at the beginning of the next. It
> > patches  normal and context mode. Side-by-side already worked in my
> > testing.
> >
> > I hope it's useful to you. Please let me know if there are changes
> > you  would like made before you can accept it.

Thank you for the contribution.
I've written a ChangeLog entry for you and attached a rebased commit.
However, the change is large enough that we'll also need a copyright
assignment from you. Can you do that?
Here are instructions:
https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.changes
[ansi-reset-less.diff (application/octet-stream, attachment)]

Information forwarded to bug-diffutils <at> gnu.org:
bug#31105; Package diffutils. (Sat, 29 Dec 2018 21:50:01 GMT) Full text and rfc822 format available.

Message #14 received at 31105 <at> debbugs.gnu.org (full text, mbox):

From: "Dennis Lambe Jr." <malsyned <at> malsyned.net>
To: Jim Meyering <jim <at> meyering.net>
Cc: 31105 <at> debbugs.gnu.org
Subject: Re: [bug-diffutils] bug#31105: bug#31105: [PATCH] Restart and reset
 ANSI escapes on each line for compatibility with less -R
Date: Sat, 29 Dec 2018 16:48:59 -0500
Quoting Jim Meyering <jim <at> meyering.net>:

> However, the change is large enough that we'll also need a copyright
> assignment from you. Can you do that?
> Here are instructions:
> https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.changes

I sent an email with the necessary information to assign <at> gnu.org

Thanks for integrating this patch

--Dennis





Information forwarded to bug-diffutils <at> gnu.org:
bug#31105; Package diffutils. (Sat, 29 Dec 2018 22:23:01 GMT) Full text and rfc822 format available.

Message #17 received at 31105 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: "Dennis Lambe Jr." <malsyned <at> malsyned.net>
Cc: 31105 <at> debbugs.gnu.org
Subject: Re: [bug-diffutils] bug#31105: bug#31105: [PATCH] Restart and reset
 ANSI escapes on each line for compatibility with less -R
Date: Sat, 29 Dec 2018 14:22:04 -0800
[Message part 1 (text/plain, inline)]
On Sat, Dec 29, 2018, 13:49 Dennis Lambe Jr. <malsyned <at> malsyned.net wrote:

>
> Quoting Jim Meyering <jim <at> meyering.net>:
>
> > However, the change is large enough that we'll also need a copyright
> > assignment from you. Can you do that?
> > Here are instructions:
> >
> https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.changes
>
> I sent an email with the necessary information to assign <at> gnu.org
>
> Thanks for integrating this patch
>

Thanks for contributing. Want to write the NEWS blurb for it?

>
[Message part 2 (text/html, inline)]

Information forwarded to bug-diffutils <at> gnu.org:
bug#31105; Package diffutils. (Sat, 29 Dec 2018 22:46:02 GMT) Full text and rfc822 format available.

Message #20 received at 31105 <at> debbugs.gnu.org (full text, mbox):

From: "Dennis Lambe Jr." <malsyned <at> malsyned.net>
To: Jim Meyering <jim <at> meyering.net>
Cc: 31105 <at> debbugs.gnu.org
Subject: Re: [bug-diffutils] bug#31105: bug#31105: [PATCH] Restart and reset
 ANSI escapes on each line for compatibility with less -R
Date: Sat, 29 Dec 2018 17:45:38 -0500
Quoting Jim Meyering <jim <at> meyering.net>:

> Thanks for contributing. Want to write the NEWS blurb for it?

Sure. I don't know if this qualifies as a bug fix, or new feature, or  
what. But how about this?

  diff --color now produces output compatible with less -R.






Information forwarded to bug-diffutils <at> gnu.org:
bug#31105; Package diffutils. (Mon, 31 Dec 2018 00:16:02 GMT) Full text and rfc822 format available.

Message #23 received at 31105 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: "Dennis Lambe Jr." <malsyned <at> malsyned.net>
Cc: 31105 <at> debbugs.gnu.org
Subject: Re: [bug-diffutils] bug#31105: bug#31105: [PATCH] Restart and reset
 ANSI escapes on each line for compatibility with less -R
Date: Sun, 30 Dec 2018 16:15:28 -0800
[Message part 1 (text/plain, inline)]
On Sat, Dec 29, 2018 at 2:45 PM Dennis Lambe Jr. <malsyned <at> malsyned.net> wrote:
...
> Sure. I don't know if this qualifies as a bug fix, or new feature, or
> what. But how about this?
>
>    diff --color now produces output compatible with less -R.

Thanks. I put that in a NEWS section labeled "Improvements."
I've made some trivial SP/TAB indentation changes in an attempt to
reach consistency (the existing code is not consistent on that front),
removed the braces you added around a single-statement "then" block,
and made this change:
-  fprintf (outfile, "\n");
+  putc ('\n', outfile);

Re TAB-vs-SP indentation, after the upcoming release, I will make the
global substitution to convert all indentation TABs to spaces.

Here's the result. Please review carefully and let me know if you
would like to change anything. I won't push until after I've heard
from you.
[diff-less-color.diff (application/octet-stream, attachment)]

Information forwarded to bug-diffutils <at> gnu.org:
bug#31105; Package diffutils. (Mon, 31 Dec 2018 04:19:01 GMT) Full text and rfc822 format available.

Message #26 received at 31105 <at> debbugs.gnu.org (full text, mbox):

From: "Dennis Lambe Jr." <malsyned <at> malsyned.net>
To: Jim Meyering <jim <at> meyering.net>
Cc: 31105 <at> debbugs.gnu.org
Subject: Re: [bug-diffutils] bug#31105: bug#31105: [PATCH] Restart and reset
 ANSI escapes on each line for compatibility with less -R
Date: Sun, 30 Dec 2018 23:18:23 -0500
Quoting Jim Meyering <jim <at> meyering.net>:

> Here's the result. Please review carefully and let me know if you
> would like to change anything. I won't push until after I've heard
> from you.

Looks great to me -- thanks!

--Dennis





Reply sent to Jim Meyering <jim <at> meyering.net>:
You have taken responsibility. (Mon, 31 Dec 2018 06:26:02 GMT) Full text and rfc822 format available.

Notification sent to "Dennis Lambe Jr." <malsyned <at> malsyned.net>:
bug acknowledged by developer. (Mon, 31 Dec 2018 06:26:02 GMT) Full text and rfc822 format available.

Message #31 received at 31105-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: "Dennis Lambe Jr." <malsyned <at> malsyned.net>
Cc: 31105-done <at> debbugs.gnu.org
Subject: Re: [bug-diffutils] bug#31105: bug#31105: [PATCH] Restart and reset
 ANSI escapes on each line for compatibility with less -R
Date: Sun, 30 Dec 2018 22:25:35 -0800
On Sun, Dec 30, 2018 at 8:18 PM Dennis Lambe Jr. <malsyned <at> malsyned.net> wrote:
> Quoting Jim Meyering <jim <at> meyering.net>:
> > Here's the result. Please review carefully and let me know if you
> > would like to change anything. I won't push until after I've heard
> > from you.
>
> Looks great to me -- thanks!

Thanks. With that, I've pushed it and am marking this as "done".




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 28 Jan 2019 12:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 5 years and 61 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.