Project

General

Profile

Actions

Feature #12913

open

A way to configure the default maximum width of pp

Added by mame (Yusuke Endoh) almost 5 years ago. Updated 4 months 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) almost 5 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) almost 5 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) almost 5 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) almost 5 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) almost 5 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) almost 5 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) 5 months 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) 5 months 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) 4 months 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) 4 months 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) 4 months 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