On Mon, Aug 22, 2022 at 11:56 AM Christopher Bläsius via Boost
I hope this is OK (I won't give a recommendation to accept or reject though).
Yes it is great! Feedback from the trenches is highly valued.
One of the more common scenarios in our applications is the construction of urls...I feel like the current API design lacks a bit in this regard.
Now that I am looking at your example code, yes I agree. I don't like some things that I'm seeing. We would be very happy to work with you and fine tune the current API to work better for your use case. To start, I would instead prefer to see your example written thusly:
auto url = boost::urls::url{}; url.set_scheme("https") .set_host("special-api.com") .set_port("443") .set_path("/prefix/api/v1/"); url.segments().append({ "applications", applicationId, "devices" }); url.params().append("since", "2022-01-01");
The url::set_... methods seem to follow a fluent api design which is nice, but unfortunately they return url_base&.
Hmm yeah I see what you mean. I didn't think of this - that's why field experience is so vital. The reason they return url_base& is because that's the class that has all the mutation logic. This is shared between `url` and `static_url`. To get functions like `set_host` to return `url&` instead would be challenging. We would have to put a copy of the entire set of mutating API member function signatures into both `url` and `static_url_base` and have those return the correct type.
So I can't construct a url inside a function call like:
httpClientGet(url{}.set_host(host).set_path("some/path"))
What if there was a helper function or a helper constructor? For example that takes the five parts and assembles them into one URL: // here, scheme, query, and fragment are `none` httpClientGet( url( none, host, "some/path", none, none );
Being able to add segments is great. Personally I'm a big fan of overloading the /-operator to append paths like in the filesystem library, because I think this results in more readable code. The example of above could become:
httpClientGet(url{host} / "some/path")
This is going to have the same problem, operator/ will return url_base&. I agree the operators are nice. We were thinking to first get some more field experience and fine tune the current APIs, and then once things are stable - pour some operators into the mix.
it is a shame that there are no semantic types to go along with them. ...The documentation states that the base url has to satisfy the absolute-URI grammar. Wouldn't it be great to have a distinct type for this?
`? I explored the use of alternative types when I first started writing the library but it became obviously fairly quickly that it
That would be a challenge to design and implement. Consider the following: absolute_uri u( "http://example.com" ); what happens if I then write: u.remove_scheme(); Does this return a new type? If the library offers uri and relative_ref as separate semantic types, what would parse_uri_reference return, given that its grammar is: URI-reference = URI / relative-ref Would the return type be `result< variant< uri_view, relative_ref_view places significant constraints on the resulting APIs, and gets quite unergonomic. Not to mention, that there would be an enormous duplication of function signatures and the documentation that goes with it. While the current design loses the compile-time distinctions between the various flavors of compliant URLs, it does gain convenience as a vocabulary type. Each container is capable of holding any valid string, and users never have to worry about the type changing upon conversion.
BTW why is there only an authority_view and not an owning container? Seems inconsistent to me.
The authority_view is a concession to users of HTTP, in order to parse request-target. The authority_view grammar is called "authority-form" in rfc7230: request-target = origin-form / absolute-form / authority-form / asterisk-form Implementing authority_view was quite simple. We made a copy of url_view and just trimmed away the stuff that wasn't relevant. But implementing an authority container with mutation operations, would not be as simple. We could have done it, and nothing stops us from adding it in the future, but I do not have sufficient evidence that such a container would be as popular or widely used as the main url containers. If enough users clamour for this feature in the future then we might add it. Generating a valid URL can be quite an undertaking. But generating a valid authority is a lot more simple. Especially since we provide the ipv4 and ipv6 address classes which will correctly format those address types for you. So while the url containers offer significant value in assisting users in producing valid URLs, we do not feel that an "authority" container would offer as much additional value beyond what the user can already accomplish by composing the authority string themselves. Furthermore there are far fewer circumstances when a user has to build an authority, compared to when they have to build a URL.
Its nice that there are functions to manually do percentage encoding, but I would prefer (additional) simpler functions which just return a std::string.
Yeah, that's fair. We have on our todo to review and refactor the percent-encoding suite of functions.
It would be nice if the library also provided the default port numbers for the schemes. I know the documentation states that this is not part of the URL-protocol, but I gonna need that information if I use the library.
That's already been added to the develop branch!
I would recommend not to start your example page with a 700 lines implementation of magnet links, that's super intimidating.
LOL! point taken...
Something I missed was a simple example showing that the parse functions also accept unknown schemes like opc.tcp:// for example.
Yes that's a good idea
Thanks a lot for all your work on providing useful high quality libraries for everyone!
Thank you for taking the time to review the library! Regards