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

get_query_param' doesn't return duplicate parameters #61

Open
m2ym opened this issue Feb 3, 2015 · 4 comments
Open

get_query_param' doesn't return duplicate parameters #61

m2ym opened this issue Feb 3, 2015 · 4 comments
Milestone

Comments

@m2ym
Copy link

m2ym commented Feb 3, 2015

# Uri.get_query_param' (Uri.of_string "/?a=1&a=2") "a";;
- : string list option = Some ["1"]

This code is expected to return Some ["1"; "2"].

@dsheets
Copy link
Member

dsheets commented Feb 3, 2015

From the mli:

(** [get_query_param' q key] returns the list of values for the
    [key] parameter in query [q].  Note that an empty list is not the
    same as a [None] return value.  For a query [foo], the mapping is:
- [/] returns None
- [/?foo] returns Some []
- [/?foo=] returns [Some [""]]
- [/?foo=bar] returns [Some ["bar"]]
- [/?foo=bar,chi] returns [Some ["bar","chi"]]

    Query keys can be duplicated in the URI, in which case the first
    one is returned.  If you want to resolve duplicate keys, obtain
    the full result set with {! query } instead.
  *)
val get_query_param' : t -> string -> string list option

I can understand the desire for a combination of all query parameters with the same name. The choice was taken in 1.x to preserve the literal structure of the URI being parsed in the interface.

Perhaps we should add a function like:

(** [get_query_params q key] returns all values found for a [key] in query [q]. *)
val get_query_params : t -> string -> string option list

Issue #17 is related to this because get_query_param does not preserve the difference between ?a and ?a=. Issue #57 is related to this because we need to offer a way to extract the literal query string and haven't, yet.

We could also offer a way to ignore the distinction between a missing value and an empty value.

What do you think?

@dsheets dsheets changed the title get_query_param' doesn't return all parameters get_query_param' doesn't return duplicate parameters Feb 3, 2015
@m2ym
Copy link
Author

m2ym commented Feb 5, 2015

The choice was taken in 1.x to preserve the literal structure of the URI being parsed in the interface.

What is the literal structure here? You mean the order of the paremeters should be preserved?

@dsheets
Copy link
Member

dsheets commented Feb 5, 2015

The order of parameters, presence/absence of =, and use as a list in a single parameter (?a=1,2,3) or multiple parameters (?a=1&a=2&a=3).

There are two reasons for this:

  1. The only reliable equality on URIs is per-character and so we would like access to parsed values that retain a similar equality; however, this doesn't prevent convenience functions.
  2. Some query-like interfaces distinguish between simple presence of keys and keys with values. For instance, http://erratique.ch/software/cmdliner/doc/Cmdliner.Arg.html#VALopt turned into a query string interface could have this behavior.

I made a design mistake back around 1.3 or so when I introduced the "handy" comma-separated list query value form. This makes collecting duplicate values for the same key harder than it should be and is almost completely unnecessary. I did it because I wanted to avoid a string option option type somewhere. It will be removed in 2.x (which I've been meaning to get to for over a year now...). Properly doing a 2.x design, clean-up, and test will probably require a couple weeks full time. I'm open to suggestions or pull requests for a 2.x branch.

@dsheets dsheets added this to the 2.0 milestone Feb 7, 2015
@m2ym
Copy link
Author

m2ym commented Feb 8, 2015

I understand the situation.

Practically, this is inconvenience because HTML form elements and jQuery's param method generate an URI containing duplicate parameters. So a function like get_query_params is necessary, in practice.

IMHO, expecting a good equality property for URIs is hard. How about introducing a concept "canonical URIs" for equality check?

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

No branches or pull requests

2 participants