Bug #3428: ri outputs ansi escape sequences even when stdout is not a tty

• Status: Closed
• Assignee: drbrain (Eric Hodel)
• Target version: 1.9.2
• ruby -v: ruby 1.9.3dev (2010-06-11 trunk 28269) [i686-linux]

[coatl (caleb clausen)]

=begin
ri should only use ansi escape sequences if explicitly asked for or if stdout is known to be a tty. Otherwise, ugliness will occur when piping ri's output to something else. For instance, here are the first few lines displayed when I type "ri exec|less":

ESC[0mESC[1;32m.execESC[m

(from ruby core)
ESC[32mImplementation from KernelESC[m

exec([env,] command... [,options])
=end

[mame (Yusuke Endoh)]

• Assignee set to drbrain (Eric Hodel)
• Target version set to 1.9.2

=begin
Hi,

2010/6/11 caleb clausen [e-mail]:

ri should only use ansi escape sequences if explicitly asked for or if stdout is known to be a tty. Otherwise, ugliness will occur when piping ri's output to something else. For instance, here are the first few lines displayed when I type "ri exec|less":

ESC[0mESC[1;32m.execESC[m

(from ruby core)
ESC[32mImplementation from KernelESC[m

?exec([env,] command... [,options])

Confirmed. "ri --help" says:

 -f, --format=NAME                Uses the selected formatter. The default
                                  formatter is bs for paged output and ansi
                                  otherwise. Valid formatters are:
                                  ansi bs html rdoc

But ansi is used for paged output in this case. So this is a bug.

Eric, could you review this patch?

diff --git a/lib/rdoc/ri/driver.rb b/lib/rdoc/ri/driver.rb
index b552335..0c04db6 100644
--- a/lib/rdoc/ri/driver.rb
+++ b/lib/rdoc/ri/driver.rb
@@ -546,7 +546,7 @@ Options may also be set in the 'RI' environment variable.

def display document
  page do |io|

•  text = document.accept formatter
  

•  text = document.accept formatter(io)
  
   io.write text
  

  end
  @@ -795,10 +795,10 @@ Options may also be set in the 'RI' environment variable.

  Creates a new RDoc::Markup::Formatter. If a formatter is given with -f,

  use it. If we're outputting to a pager, use bs, otherwise ansi.

• def formatter

• def formatter(io)
  if @formatter_klass then
  @formatter_klass.new

• elsif paging? then

• elsif paging? or !io.tty? then
  RDoc::Markup::ToBs.new
  else
  RDoc::Markup::ToAnsi.new

--
Yusuke Endoh [e-mail]
=end

[kosaki (Motohiro KOSAKI)]

=begin
ruby 1.8.x don't have this issue. in other word, this is regression.
=end

[coatl (caleb clausen)]

=begin
Ruby 1.8's ri never prints any escape sequences... at least, I know that's true in 1.8.6.
=end

[mame (Yusuke Endoh)]

=begin
Eric,

May I commit my patch?

--
Yusuke Endoh [e-mail]
=end

[mame (Yusuke Endoh)]

=begin
I don't care anymore.
I'll decide tickets about rubygems at my discretion of assistant
release manager.

Anyone, give him a kick.

--
Yusuke Endoh [e-mail]
=end

[mame (Yusuke Endoh)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

=begin
This issue was solved with changeset r28455.
caleb, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

=end

[coatl (caleb clausen)]

=begin
Since the maintainer of rdoc is AWOL, I feel we really need to have a test for this bug, so that the next time eric merges his latest rdoc into ruby, we can be sure he didn't accidentally overwrite this fix and cause a regression.

If I submit a patch to test this bug, will it be accepted?
=end

[mame (Yusuke Endoh)]

=begin
Hi,

Since the maintainer of rdoc is AWOL, I feel we really need to have a test for this bug, so that the next time eric merges his latest rdoc into ruby, we can be sure he didn't accidentally overwrite this fix and cause a regression.

I committed a test at r28460. It is very trivial, though.

If I submit a patch to test this bug, will it be accepted?

Of course!

--
Yusuke Endoh [e-mail]
=end

[coatl (caleb clausen)]

• File rdoc_ansi_esc_test.diff rdoc_ansi_esc_test.diff added

=begin
Sorry for the delay getting back to you on this. Here's a patch which expands test_formatter slightly to exercise the path when output is not a tty. I had hoped to write a high-level black box test, which would be more robust and thorough. But that has proved to be a little more involved than I anticipated.
=end

[mame (Yusuke Endoh)]

=begin
Committed to trunk at r28611. Thanks!

2010/7/11 caleb clausen [e-mail]:

Issue #3428 has been updated by caleb clausen.

File rdoc_ansi_esc_test.diff added

Sorry for the delay getting back to you on this. Here's a patch which expands test_formatter slightly to exercise the path when output is not a tty. I had hoped to write a high-level black box test, which would be more robust and thorough. But that has proved to be a little more involved than I anticipated.

--
Yusuke Endoh [e-mail]

=end