Project

General

Profile

Actions

Feature #12913

open

A way to configure the default maximum width of pp

Added by mame (Yusuke Endoh) over 4 years ago. Updated 8 days ago.

Status:
Assigned
Priority:
Normal
Target version:
-
[ruby-core:78062]

Description

How about having an easy way to configure the maximum width of a line of pp output?
Currently, pp accepts the maximum width as an optional argument:

pp(big_array, $>, 120)

However, this is obviously too long for a useful debugging-purpose method like pp. Even worse, we must add the fragment ", $>, 120" to all calls to pp. I don't feel this is reasonable.

The patch attached provides PP.default_maxwidth= and PP.default_maxwidth, which can be used to configure the default setting of the maxwidth.

PP.default_maxwidth = 1
pp([1, 2, 3])
#=> [1,
#    2,
#    3]

Akr-san, what do you think?


Files

pp-default-maxwidth.patch (1.05 KB) pp-default-maxwidth.patch mame (Yusuke Endoh), 11/09/2016 02:59 PM

Updated by akr (Akira Tanaka) over 4 years ago

I think the columns obtained by IO#winsize of io/console is better default now.

I'm not sure you are satisfied with it, though.

Updated by mame (Yusuke Endoh) over 4 years ago

Akr-san, thank you for your reply.

I agree that it is a better default than the fixed 79.

However, I'd still like to have PP.default_maxwidth= too.
My motivation is to show abstract syntax tree (in my article).
In such a case, I think that "break as far as possible" would be useful.

Thank you for your consideration,

Actions #3

Updated by akr (Akira Tanaka) over 4 years ago

  • Status changed from Open to Feedback

Yusuke Endoh wrote:

In such a case, I think that "break as far as possible" would be useful.

"break as far as possible" is opposite to pp.rb's intent:
"don't break as far as possible (in screen width limitation)".

I think a global configuration to choose different intent for single API is not a good idea.

pp (including pretty_inspect and PP.pp) method is used in many gems.
I think the intent of most of them is "don't break as far as possible".
PP.default_maxwidth = 1 breaks the intent.

I recommend that you define a method to call PP.pp with width = 1.

Updated by mame (Yusuke Endoh) over 4 years ago

Akira Tanaka wrote:

pp (including pretty_inspect and PP.pp) method is used in many gems.

Note that my proposal is only for PP; the scope does not include pretty_inspect.

PP is a debugging-purpose API, so it is rarely used in a released code. So, we don't have to consider the case where PP is used in multiple modules simultaneously, I think. Also, because of debugging-purpose, it would be good to prioritize usability than safety.

Updated by akr (Akira Tanaka) over 4 years ago

I think pp is not only for debug.
I guess someone use it for logging, for example.

So, I think global configuration is dangerous,

Updated by mame (Yusuke Endoh) over 4 years ago

  • Status changed from Feedback to Rejected

Okay, I respect your decision. Thank you for your time.

Updated by nobu (Nobuyoshi Nakada) over 3 years ago

What about this?

  • affects PP.pp and Kernel#pp only
  • try console window size, COLUMNS environment variable, then old good 80
diff --git a/lib/pp.rb b/lib/pp.rb
indiff --git a/lib/pp.rb b/lib/pp.rb
index 85401c8aa6..a364d0fc2a 100644
--- a/lib/pp.rb
+++ b/lib/pp.rb
@@ -68,7 +68,7 @@
   # If +width+ is omitted, 79 is assumed.
   #
   # PP.pp returns +out+.
-  def PP.pp(obj, out=$>, width=79)
+  def PP.pp(obj, out=$>, width=PP.width_for(out))
     q = PP.new(out, width)
     q.guard_inspect_key {q.pp obj}
     q.flush
@@ -91,6 +91,15 @@
   def PP.mcall(obj, mod, meth, *args, &block)
     mod.instance_method(meth).bind(obj).call(*args, &block)
   end
+
+  def PP.width_for(out)
+    begin
+      require 'io/console'
+      _, width = out.winsize
+    rescue LoadError, NoMethodError, Errno::ENOTTY
+    end
+    (width || ENV['COLUMNS']&.to_i&.nonzero? || 80) - 1
+  end
   # :startdoc:

   @sharing_detection = false
@@ -549,9 +558,9 @@
   # prints arguments in pretty form.
   #
   # pp returns argument(s).
-  def pp(*objs)
+  def pp(*objs, out: $>, width: PP.width_for(out))
     objs.each {|obj|
-      PP.pp(obj)
+      PP.pp(obj, out, width)
     }
     objs.size <= 1 ? objs.first : objs
   end

Updated by akr (Akira Tanaka) over 3 years ago

nobu (Nobuyoshi Nakada) wrote:

What about this?

  • affects PP.pp and Kernel#pp only
  • try console window size, COLUMNS environment variable, then old good 80

It seems fine.

Updated by mame (Yusuke Endoh) about 1 month ago

  • Assignee changed from akr (Akira Tanaka) to nobu (Nobuyoshi Nakada)
  • Status changed from Rejected to Assigned

akr (Akira Tanaka) wrote in #note-8:

nobu (Nobuyoshi Nakada) wrote:

What about this?

  • affects PP.pp and Kernel#pp only
  • try console window size, COLUMNS environment variable, then old good 80

It seems fine.

I prefer nobu (Nobuyoshi Nakada)'s patch to mine. Since it was fine to akr (Akira Tanaka), how about committing it?

Updated by nobu (Nobuyoshi Nakada) 30 days ago

https://github.com/nobu/ruby/runs/2672425246?check_suite_focus=true#step:15:239
A test in rbs passes keyword arguments to pp.
Also ko1 (Koichi Sasada) says he uses keywords arguments with pp very often.

Updated by mame (Yusuke Endoh) 18 days ago

So, some people (including ko1 (Koichi Sasada)) have a custom to pass keyword arguments to Kernel#pp:

foo = 42
bar = 43
pp(foo: foo, bar: bar) #=> {:foo=>42, :bar=>43}

Unfortunately, introducing keyword arguments to Kernel#pp breaks their custom.

How about introducing pp_out and pp_width keywords? It is a bit dirty, but good enoguh.

diff --git a/lib/pp.rb b/lib/pp.rb
index 72480e5304..b090d2bdeb 100644
--- a/lib/pp.rb
+++ b/lib/pp.rb
@@ -91,6 +91,15 @@ def PP.singleline_pp(obj, out=$>)
   def PP.mcall(obj, mod, meth, *args, &block)
     mod.instance_method(meth).bind_call(obj, *args, &block)
   end
+
+  def PP.width_for(out)
+    begin
+      require 'io/console'
+      _, width = out.winsize
+    rescue LoadError, NoMethodError, Errno::ENOTTY
+    end
+    (width || ENV['COLUMNS']&.to_i&.nonzero? || 80) - 1
+  end
   # :startdoc:

   if defined? ::Ractor
@@ -596,12 +605,21 @@ def pretty_inspect
     PP.pp(self, ''.dup)
   end

-  # prints arguments in pretty form.
+  # Prints arguments in pretty form.
   #
   # pp returns argument(s).
-  def pp(*objs)
+  #
+  # It prints to +$>+ by default. A keyword argument +pp_out+ is given,
+  # it is used as the output target.
+  #
+  # The output is fitted to the width of the console by default.
+  # A keyword +pp_width+ is used if given.
+  def pp(*objs, **kw)
+    out = kw.delete(:pp_out) || $>
+    width = kw.delete(:pp_width) || PP.width_for(out)
+    objs << kw unless kw.empty?
     objs.each {|obj|
-      PP.pp(obj)
+      PP.pp(obj, out, width)
     }
     objs.size <= 1 ? objs.first : objs
   end
diff --git a/prelude.rb b/prelude.rb
index b1e477a3ea..b8c564e84e 100644
--- a/prelude.rb
+++ b/prelude.rb
@@ -10,9 +10,9 @@ def irb
 end

 module Kernel
-  def pp(*objs)
+  def pp(...)
     require 'pp'
-    pp(*objs)
+    pp(...)
   end

   # suppress redefinition warning

Updated by mame (Yusuke Endoh) 8 days ago

We discussed this issue in today's dev-meeting. akr (Akira Tanaka) suggested that

  1. To make the default size to be io/console's winsize (width)
  2. Instead of changing Kernel#pp, extend PP.pp to accept keywords out and width
  3. If we want width=1, define another new method like PP.fully_expanded_pp(obj) or something (Note that there is already PP.singleline_pp(obj))

and we agreed with them. I'll commit a change for 1 later.

I am happy to create another patch for 2 and 3. Anyone have an idea about the name of fully_expanded_pp?

Updated by jeremyevans0 (Jeremy Evans) 8 days ago

mame (Yusuke Endoh) wrote in #note-12:

We discussed this issue in today's dev-meeting. akr (Akira Tanaka) suggested that

  1. To make the default size to be io/console's winsize (width)
  2. Instead of changing Kernel#pp, extend PP.pp to accept keywords out and width
  3. If we want width=1, define another new method like PP.fully_expanded_pp(obj) or something (Note that there is already PP.singleline_pp(obj))

and we agreed with them. I'll commit a change for 1 later.

I am happy to create another patch for 2 and 3. Anyone have an idea about the name of fully_expanded_pp?

In keeping with the spirit that Kernel#pp is more expanded than Kernel#p, I suggest PP.ppp. :)

Actions

Also available in: Atom PDF