Skip to content

Add Socket::Address#pretty_print and #inspect#6704

Merged
RX14 merged 2 commits intocrystal-lang:masterfrom
straight-shoota:jm/feature/socket_address-pretty_print
Sep 12, 2018
Merged

Add Socket::Address#pretty_print and #inspect#6704
RX14 merged 2 commits intocrystal-lang:masterfrom
straight-shoota:jm/feature/socket_address-pretty_print

Conversation

@straight-shoota
Copy link
Copy Markdown
Member

No description provided.

@asterite
Copy link
Copy Markdown
Member

Why? How does it look if you print an array of addresses?

@straight-shoota
Copy link
Copy Markdown
Member Author

straight-shoota commented Sep 12, 2018

p [Socket::IPAddress.new("127.0.0.1", 80), Socket::IPAddress.new("::1", 8000)]
#before:
# => [Socket::IPAddress(@family=INET, @size=16, @port=80, @address="127.0.0.1", @addr6=nil, @addr4=LibC::InAddr(@s_addr=16777343_u32)), Socket::IPAddress(@family=INET6, @size=28, @port=8000, @address="::1", @addr6=LibC::In6Addr(@__in6_u=LibC::In6AddrIn6U(@__u6_addr8=StaticArray[0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 0_u8, 1_u8], @__u6_addr16=StaticArray[0_u16, 0_u16, 0_u16, 0_u16, 0_u16, 0_u16, 0_u16, 256_u16], @__u6_addr32=StaticArray[0_u32, 0_u32, 0_u32, 16777216_u32])), @addr4=nil)]

# after:
# => [127.0.0.1:80, [::1]:8000]

I don't think there is any reason for #inspect to print all the internal values when the string representation conveys exactly the same information in a human readable format.

@asterite
Copy link
Copy Markdown
Member

I agree. It should print something like IpAddress(127.0.0.1:80) but definitely not just the address.

@asterite
Copy link
Copy Markdown
Member

Not sure Ruby has a class for ip addresses, but it would be interesting to see what Ruby does here.

@asterite
Copy link
Copy Markdown
Member

Here's Ruby:

reqirb(main):001:0> require 'ipaddr'
=> true
irb(main):002:0> ipaddr1 = IPAddr.new "3ffe:505:2::1"
=> #<IPAddr: IPv6:3ffe:0505:0002:0000:0000:0000:0000:0001/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff>
irb(main):003:0> ipaddr3 = IPAddr.new "192.168.2.0/24"
=> #<IPAddr: IPv4:192.168.2.0/255.255.255.0>
irb(main):004:0> ipaddr3 = IPAddr.new "192.168.2.0"
=> #<IPAddr: IPv4:192.168.2.0/255.255.255.255>

As I mentioned in some other PR, inspect should produce an output that's clearly delimited. But "[1]::2]" looks a bit like a broken array literal.

@straight-shoota
Copy link
Copy Markdown
Member Author

There is IPAddr but it's quite a bit different from Crystal's IPAddress which is essentially a socket address consisting of IP and port. In Ruby, it's only an IP address but with subnet mask.

p IPAddr.new "3ffe:505:2::1" # => #<IPAddr: IPv6:3ffe:0505:0002:0000:0000:0000:0000:0001/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff>

@straight-shoota
Copy link
Copy Markdown
Member Author

You're right. Let's make it Socket::IPAddress(127.0.0.1:80).

@RX14
Copy link
Copy Markdown
Member

RX14 commented Sep 12, 2018

Aren't we throwing away a lot of the information here?

@asterite
Copy link
Copy Markdown
Member

@straight-shoota Oh, and to_s should behave like what you originally proposed for inspect.

Again, in Ruby:

$ irb
irb(main):001:0> require 'ipaddr'
=> true
irb(main):002:0> puts IPAddr.new "192.168.2.0"
192.168.2.0

@straight-shoota
Copy link
Copy Markdown
Member Author

@RX14 Not really. All the information is expressed in the string representation. The data structures are just pretty complicated which makes it look like there is a whole lot more information in it.

@asterite #to_s has been like this before and it doesn't change with this PR. It's even used by inspect.
Additionally to the Ruby string representation, there is also a port.

@straight-shoota
Copy link
Copy Markdown
Member Author

Maybe we should really rename IPAddress to IPSocketAddress or SocketAddress to make it more clear, that it is not just an IP address.

@RX14 RX14 merged commit ee71249 into crystal-lang:master Sep 12, 2018
@RX14 RX14 added this to the 0.27.0 milestone Sep 12, 2018
@straight-shoota straight-shoota deleted the jm/feature/socket_address-pretty_print branch September 12, 2018 20:47
ezrast pushed a commit to ezrast/crystal that referenced this pull request Oct 2, 2018
* Add Socket::IPAddress#pretty_print and #inspect

* fixup! Add Socket::IPAddress#pretty_print and #inspect
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.

4 participants