Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pretty print #3588

Merged
merged 1 commit into from
Nov 27, 2016
Merged

Add pretty print #3588

merged 1 commit into from
Nov 27, 2016

Conversation

asterite
Copy link
Member

This adds pretty printing to Crystal objects. And now when you do p obj that actually pretty prints obj instead of using inspect. The reason for this is that I always find Ruby's pp better whenever I want to quickly "inspect" an object, so I think pretty printing should be preferred. There's also Elixir, which only has IO.inspect which does pretty printing, and that never bothered me.

Example:

class Point
  def initialize
    @x = 1
    @y = 2
    @z = 3
  end
end

class Foo
  @points : Array(Point)

  def initialize
    @points = [Point.new] * 3
  end
end

class Bar
  def initialize
    @foo = Foo.new
  end
end

bar = Bar.new
p bar

Output before:

#<Bar:0x103173f00 @foo=#<Foo:0x103173ee0 @points=[#<Point:0x103173ea0 @x=1, @y=2, @z=3>, #<Point:0x103173ea0 @x=1, @y=2, @z=3>, #<Point:0x103173ea0 @x=1, @y=2, @z=3>]>>

Output after:

#<Bar:0x109f16f00
 @foo=
  #<Foo:0x109f16ee0
   @points=
    [#<Point:0x109f16ea0 @x=1, @y=2, @z=3>,
     #<Point:0x109f16ea0 @x=1, @y=2, @z=3>,
     #<Point:0x109f16ea0 @x=1, @y=2, @z=3>]>>

I think I prefer the later to be outputted, always (one can do puts obj.inspect to get the old behaviour)

I'll be honest: I read the papers about pretty printing and did an implementation myself, but then I checked how it was done in Ruby and it was much better. The difference is that Ruby streams directly to the given output whenever possible, while my implementation constructed the whole tree and then outputted. In the end, my implementation consumed a lot of memory... so I decided to first port Ruby's code to Crystal, check if it works, and then try to understand it. Not surprisingly, porting the code was extremely easy! And it worked out of the box. I think I understand now how it works, though the algorithm is pretty complex (if somebody would like to add more comments afterwards, please do so!). As always, I don't know if I should do something about the copyright, the algorithms seem to be public though I'm not so sure about Ruby's code.

Now we'll have to_s, inspect and pretty_print... maybe we can remove inspect? Or maybe we can declare it such that it creates a pretty printer that will output everything into a single line and then invoke pretty_print with it? In that way users would only need to override to_s and/or pretty_print, but never inspect. In any case, maybe that can be decided after we merge this PR.

@sdogruyol
Copy link
Member

👍 for removing inspect

@RX14
Copy link
Member

RX14 commented Nov 26, 2016

I'm in favour of keeping inspect, but making it delegate to pretty_print by default.

@kostya
Copy link
Contributor

kostya commented Nov 26, 2016

why remove inspect?
this is useful when write some object into log LOG.info "... #{obj.inspect} ...", and pretty_print would be bad in log, because of newlines. and to_s method can do something else on the object.

in ruby actually "#{}" already call inspect

@Sija
Copy link
Contributor

Sija commented Nov 26, 2016

I'd keep inspect too, its output is meant to be more terse than pretty_print (for debug purposes) and quite often may be different than to_s.

@@ -279,12 +279,12 @@ describe "Hash" do
end

describe "to_s" do
assert { {1 => 2, 3 => 4}.to_s.should eq("{1 => 2, 3 => 4}") }
assert { {1 => 2, 3 => 4}.to_s.should eq("{1=>2, 3=>4}") }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this, I would prefer if by default the generated representation is valid and properly formatted Crystal code, so paste the output around doesn't trigger formatter differences?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good

Verified

This commit was signed with the committer’s verified signature. The key has expired.
nomadium Miguel Landaeta
@asterite asterite force-pushed the feature/pretty_print branch from 58dbfa8 to f36d122 Compare November 27, 2016 13:28
@asterite asterite added this to the 0.20.1 milestone Nov 27, 2016
@asterite asterite merged commit 84b0c6d into master Nov 27, 2016
@asterite asterite deleted the feature/pretty_print branch November 27, 2016 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants