> On 31 May 2024, at 12:32, José Valim <jose.va...@dashbit.co> wrote:
>
> I have reviewed the pull request and I noticed that it does validate the URL
> before generating URI.new!, but it now has the unfortunate side-effect that
> you may have 2 (or 3) distinct representations when inspecting, which is
> another downside of the proposal. I don't think the cons (multiple
> representations, hiding of structures) justify the pro of a more succinct
> representation.
I agree, 3 representations is probably too much, I think having just 2,
URI.new! and %URI{}, is more palatable. This reminds me that we used to print
`%{~D[2024-01-01] | calendar: Calendar.Holocene}` as `%Date{year: 2024, month:
1, day: 1, calendar: Calendar.Holocene}` so FWIW there was some precedent for
this. (It has been improved since, though, and nowadays it would have printed:
`~D[2024-01-01 Calendar.Holocene]`.)
If discoverability is more important than verbosity then that’s that. I
personally don’t think it matters that much, we have good error messages when
mistyping field names and we have the |> inspect(structs: false) and IEx.Info
“fallbacks”. Here’s somewhat contrived example so take it for what it is but
the difference is pretty stark:
```
iex> Enum.map(1..5, &URI.new!("https://example.com/foo?x=#{&1}";))
[URI.new!("https://example.com/foo?x=1";),
URI.new!("https://example.com/foo?x=2";),
URI.new!("https://example.com/foo?x=3";),
URI.new!("https://example.com/foo?x=4";),
URI.new!("https://example.com/foo?x=5";)]
```
```
iex> Enum.map(1..5, &URI.new!("https://example.com/foo?x=#{&1}";))
[
%URI{
scheme: "https",
userinfo: nil,
host: "example.com",
port: 443,
path: "/foo",
query: "x=1",
fragment: nil
},
%URI{
scheme: "https",
userinfo: nil,
host: "example.com",
port: 443,
path: "/foo",
query: "x=2",
fragment: nil
},
%URI{
scheme: "https",
userinfo: nil,
host: "example.com",
port: 443,
path: "/foo",
query: "x=3",
fragment: nil
},
%URI{
scheme: "https",
userinfo: nil,
host: "example.com",
port: 443,
path: "/foo",
query: "x=4",
fragment: nil
},
%URI{
scheme: "https",
userinfo: nil,
host: "example.com",
port: 443,
path: "/foo",
query: "x=5",
fragment: nil
}
]
```
Can’t say I run into stuff like this very often but on the rare occasion I
wouldn’t mind something way more compact so I could focus on what matters (i.e.
not the internal structure)
Anyway, thanks for considering this!
>
> On Fri, May 31, 2024 at 12:20 PM José Valim <jose.va...@dashbit.co
> <mailto:jose.va...@dashbit.co>> wrote:
>> There are some issues with this proposal:
>>
>> 1. URI parse does not validate, it only parses the components, which means
>> inspecting with URI.new! after URI.parse! does not guarantee it will be
>> parseable and return the same result back.
>>
>> 2. URI.new! (and similar) hide the underlying fields of the URI, making them
>> harder to discover. I believe URI.new! (and similar) are almost always
>> justified when the structs fields are private, which is not the case here,
>> so there is more space for debate
>>
>> If the main goal is to reduce verbosity, then maybe we should consider
>> tagging all fields as optional, so we have this instead:
>>
>> iex> URI.new!("https://elixir-lang.org <https://elixir-lang.org/>")
>> %URI{
>> scheme: "https",
>> host: "elixir-lang.org <http://elixir-lang.org/>",
>> port: 443
>> }
>>
>> It may be an acceptable trade-off between hiding fields and keeping it more
>> compact.
>>
>> On Fri, May 31, 2024 at 12:13 PM Wojtek Mach <woj...@wojtekmach.pl
>> <mailto:woj...@wojtekmach.pl>> wrote:
>>> The current inspect implementation is pretty verbose:
>>>
>>> ```elixir
>>> iex> URI.new!("https://elixir-lang.org <https://elixir-lang.org/>")
>>> %URI{
>>> scheme: "https",
>>> userinfo: nil,
>>> host: "elixir-lang.org <http://elixir-lang.org/>",
>>> port: 443,
>>> path: nil,
>>> query: nil,
>>> fragment: nil
>>> }
>>> ```
>>>
>>> I'd like to propose a more concise one along the lines of "Expression-based
>>> inspection" from ELixir v1.14
>>> <https://github.com/elixir-lang/elixir/blob/v1.14/CHANGELOG.md#expression-based-inspection-and-inspect-improvements>:
>>>
>>> ```elixir
>>> iex> URI.new!("https://elixir-lang.org <https://elixir-lang.org/>")
>>> URI.new!("https://elixir-lang.org <https://elixir-lang.org/>")
>>> ```
>>>
>>> There is a subtle difference between `URI.new!/1` and `URI.parse/1`, the
>>> former sets the deprecated `authority` field to `nil` so this proposal
>>> takes that into consideration, returns `URI.parse` "as is":
>>>
>>> ```elixir
>>> iex> URI.parse("https://elixir-lang.org <https://elixir-lang.org/>")
>>> URI.parse("https://elixir-lang.org <https://elixir-lang.org/>")
>>> ```
>>>
>>> `URI.new!/1` is stricter than `URI.parse/1` and in particular it does not
>>> allow non-escaped characters, notably `"` and `(` and `)` which would
>>> conflict with the inspect representation so for these I propose to return
>>> `%URL{}`:
>>>
>>> ```elixir
>>> iex> URI.parse("https://elixir-lang.org/\ <https://elixir-lang.org/%5C>"")
>>> %URI{
>>> scheme: "https",
>>> authority: "elixir-lang.org <http://elixir-lang.org/>",
>>> userinfo: nil,
>>> host: "elixir-lang.org <http://elixir-lang.org/>",
>>> port: 443,
>>> path: "/\"",
>>> query: nil,
>>> fragment: nil
>>> }
>>> ```
>>>
>>> I vaguely remember previous discussions about this and I believe the
>>> biggest concern was "hiding" the internal structure. For example, what is
>>> the `:scheme`, `:host` and `:path` in intentionally mistyped URL
>>> `http:/foo`? We would not get the answer from Inspect:
>>>
>>> ```elixir
>>> iex> URI.new!("http:/foo")
>>> URI.new!("http:/foo")
>>> ```
>>>
>>> but I'd like to propose to additionally implement `IEx.Info`:
>>>
>>> ```
>>> iex> i URI.new!("http:/foo")
>>> Term
>>> URI.new!("http:/foo")
>>> Data type
>>> URI
>>> Description
>>> This is a struct representing a URI.
>>> Raw representation
>>> %URI{scheme: "http", authority: nil, userinfo: nil, host: nil, port: 80,
>>> path: "/foo", query: nil, fragment: nil}
>>> Reference modules
>>> URI
>>> Implemented protocols
>>> IEx.Info, Inspect, String.Chars
>>> ```
>>>
>>> PR: https://github.com/elixir-lang/elixir/pull/13623
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "elixir-lang-core" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an
>>> email to elixir-lang-core+unsubscr...@googlegroups.com
>>> <mailto:elixir-lang-core+unsubscr...@googlegroups.com>.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/elixir-lang-core/e36197aa-1b77-4000-a3b2-55f022e20ce1n%40googlegroups.com
>>>
>>> <https://groups.google.com/d/msgid/elixir-lang-core/e36197aa-1b77-4000-a3b2-55f022e20ce1n%40googlegroups.com?utm_medium=email&utm_source=footer>.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "elixir-lang-core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to elixir-lang-core+unsubscr...@googlegroups.com
> <mailto:elixir-lang-core+unsubscr...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4J2z%2B4LmR49nehE_GD8qeZye%3D867PfGuvTcf%2BSzWcyPtg%40mail.gmail.com
>
> <https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4J2z%2B4LmR49nehE_GD8qeZye%3D867PfGuvTcf%2BSzWcyPtg%40mail.gmail.com?utm_medium=email&utm_source=footer>.
--
You received this message because you are subscribed to the Google Groups
"elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/elixir-lang-core/FC8602E3-CE2E-4F6D-9873-A16CB736080B%40wojtekmach.pl.
