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.
On Fri, May 31, 2024 at 12:20 PM José Valim <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";)
> %URI{
> scheme: "https",
> host: "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> wrote:
>
>> The current inspect implementation is pretty verbose:
>>
>> ```elixir
>> iex> URI.new!("https://elixir-lang.org";)
>> %URI{
>> scheme: "https",
>> userinfo: nil,
>> host: "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";)
>> URI.new!("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";)
>> URI.parse("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/\"";)
>> %URI{
>> scheme: "https",
>> authority: "elixir-lang.org",
>> userinfo: nil,
>> host: "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.
>> 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.
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.
