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 a new() primitive to the uri package. #619

Open
vxern opened this issue May 31, 2024 · 5 comments
Open

Add a new() primitive to the uri package. #619

vxern opened this issue May 31, 2024 · 5 comments

Comments

@vxern
Copy link

vxern commented May 31, 2024

Inspired by the request.new() and response.new() functions from the gleam_http package, and while working with Uris, I thought that it would also be a good idea to be able to do the same with Uri.

Provisionally, the implementation of the function would look as follows:

pub fn new() -> Uri {
  Uri(
    scheme: None,
    userinfo: None,
    host: None,
    port: None,
    path: "",
    query: None,
    fragment: None,
  )
}

This would create a completely bare Uri object that the developer could then go ahead and build on top of. I have created a related issue here that would implement 'builder' primitives to set fields on the Uri object. Coupled, these primitives would create a really nice pattern for building out a Uri, but even without those 'setter' primitives being present yet, the developer could still utilise this implementation of new() as follows:

let uri = Uri(..uri.new(), scheme: "http", host: "localhost")

This primitive would make cases like the following taken from uri_test.gleam more concise:

uri.Uri(Some("ftp"), None, None, None, "", None, None)
|> uri.to_string
|> should.equal("ftp:")

allowing them to be switched out for:

uri.Uri(..uri.new(), scheme: Some("ftp"))
|> uri.to_string
|> should.equal("ftp:")

and coupled with the 'setter' primitive proposal mentioned earlier (here), if that proposal were to be implemented:

uri.new()
|> uri.set_scheme("ftp")
|> uri.to_string
|> should.equal("ftp:")

If okay'd, I'd be happy to go ahead and implement this myself.

@chuckwondo
Copy link
Contributor

If I'm reading the URI syntax description correctly, it seems that at a minimum scheme and path are required in order to have a minimally valid URI. If that is indeed correct, then I would think that new might arguably be a bitter better implemented as follows, as I don't imagine it would make sense to not specify a scheme:

pub fn new(scheme: String) -> Uri {
  Uri(
    scheme: Option(scheme),
    userinfo: None,
    host: None,
    port: None,
    path: "",
    query: None,
    fragment: None,
  )
}

Your examples might then become slightly simpler. For example:

uri.new("ftp")
|> uri.to_string
|> should.equal("ftp:")

Arguably, defaulting path to an empty string likely doesn't make sense either since that's equivalent to no path at all, which, again, seems to be invalid, but URI validation is a big can of worms to deal with, so perhaps better left for a separate discussion.

@lpil
Copy link
Member

lpil commented Jul 25, 2024

That sounds sensible to me!

That said, given we're unlikely to get the builder API is there still a use case for this function?

@chuckwondo
Copy link
Contributor

That sounds sensible to me!

That said, given we're unlikely to get the builder API is there still a use case for this function?

Probably not.

@vxern
Copy link
Author

vxern commented Jul 25, 2024

Even if we do not have builders, would it not be valuable to still have a function (or perhaps even a constant Uri.empty) that would eliminate the boilerplate needed to create a complete Uri?

Referring back to this usage:

let uri = Uri(..uri.new(), scheme: "http", host: "localhost")

@lpil
Copy link
Member

lpil commented Jul 26, 2024

OK, sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants