Thanks for the thoughtful feedback. I think we should create URI.append_query/1 and specify in documentation that duplicate keys are not considered and that it uses URI.encode_query for encoding. I believe that "append" as used in Tupple.append/2 is clear enough to indicate key duplication without having to read the documentation.
If we want to address updating query params we could also create URI.put_query/3 and URI.put_new_query/3 which would follow the same logic and naming implications of both the Map and Keyword module functions where "put" adds and updates if it exists, while "put_new" only adds if it does not exist. You raise a good point about the possible limitations of URI.encode_query/1 but I think that should be solved as a separate issue. URI.encode_query/1 is currently the only option in the URI module, so I don't see any harm in using it internally in another URI function. Maybe one solution could be to augment URI.encode_query/1 with options that could also be passed into the "put" and "put_new" functions. Overall, I think that the complexity of URI.encode_query/1 could be address in another request for change. There is a significant value add in at least adding URI.append_query/1 to the API to allow developers to stay within the URI abstraction when building a URI and to push the URI module in the direction of a subject as first argument module. On Friday, November 6, 2020 at 5:49:32 AM UTC-7 José Valim wrote: > Hi Spencer, thanks for the proposal. > > My main question is what happens if the URI already has a query. Do we > update it? Do we replace it? On this question alone, if we were to add such > functionality, then I think it should not be called encode_query but rather > update_query/replace_query. > > update_query itself has ambiguous connotations as well, in that it can > either update_query (replacing duplicate keys) or simply append to it since > a query can have duplicate keys (so maybe it should be called > append_query?). Note how a web server will handle duplicate keys is > completely up to the web server implementation, so having an implementation > that appends by default can lead to surprising behaviour. > > The implementation of the PR follows the append_query semantics - but we > need to discuss all of those things if we are to accept such a proposal > because these semantics need to make sense in the general case. > > The other thing to note is that "URI.encode_query" is one of many ways of > encoding a query string. Web applications have historically built more > complex query encoding/decoding schemas that (poorly) support richer data > structure, so this API may not be enough depending on what you want to > encode. > > > > On Fri, Nov 6, 2020 at 1:18 PM Bruce Tate <[email protected]> wrote: > >> I think the pull request is a much better API. It both takes and returns >> a URI, and also gains type checking benefits (and readability benefits... >> single level of abstraction). >> >> -bt >> >> On Fri, Nov 6, 2020 at 3:16 AM Andrea Leopardi <[email protected]> >> wrote: >> >>> Hey Spencer, >>> >>> Considering how concise the snippet of code you showed is even without >>> URI.encode_query/2, I think the value of having a smaller API surface is >>> higher than the value of adding a function that you can replace with just a >>> few more characters. Thoughts? >>> >>> On Thu, Nov 5, 2020 at 7:28 PM [email protected] < >>> [email protected]> wrote: >>> >>>> I would like to propose the addition of URI.encode_query/2 to the URI >>>> module to facilitate URI construction in a pipe friendly fashion. >>>> >>>> Here is an example MR on my fork of elixir >>>> https://github.com/spencerdcarlson/elixir/pull/2 >>>> >>>> -- >>>> 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 [email protected]. >>>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/elixir-lang-core/7e297574-b4d0-4b03-8279-4abe50c191e0n%40googlegroups.com >>>> >>>> <https://groups.google.com/d/msgid/elixir-lang-core/7e297574-b4d0-4b03-8279-4abe50c191e0n%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 [email protected]. >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/elixir-lang-core/CAM9Rf%2BKkTGR%3DG2bRApfhr51XD8RHr1HbZZg-Ef%3DJ8rb5aOOR5A%40mail.gmail.com >>> >>> <https://groups.google.com/d/msgid/elixir-lang-core/CAM9Rf%2BKkTGR%3DG2bRApfhr51XD8RHr1HbZZg-Ef%3DJ8rb5aOOR5A%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> >> >> >> -- >> >> Regards, >> Bruce Tate >> CEO >> >> >> <https://bowtie.mailbutler.io/tracking/hit/f8218219-d2a8-4de4-9fef-1cdde6e723f6/c7c97460-016e-45fb-a4ab-0a70318c7b97> >> >> Groxio, LLC. >> 512.799.9366 <(512)%20799-9366> >> [email protected] >> grox.io >> >> -- >> 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 [email protected]. >> > To view this discussion on the web visit >> https://groups.google.com/d/msgid/elixir-lang-core/CAFXvW-4KRBnv3VLO3S6OSBSgLOHOKKohgee%3DqPo5ZZFxVfs1LA%40mail.gmail.com >> >> <https://groups.google.com/d/msgid/elixir-lang-core/CAFXvW-4KRBnv3VLO3S6OSBSgLOHOKKohgee%3DqPo5ZZFxVfs1LA%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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/ce7629f9-547e-4477-8141-2cdf18fc8daan%40googlegroups.com.
