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

interface is inconsistent #18

Open
dsheets opened this issue May 19, 2013 · 26 comments
Open

interface is inconsistent #18

dsheets opened this issue May 19, 2013 · 26 comments

Comments

@dsheets
Copy link
Member

dsheets commented May 19, 2013

functions with primes shouldn't have, functions without should; get_query_param* is the current exception

@dsheets dsheets added this to the 2.0 milestone Feb 7, 2015
@rgrinberg
Copy link
Member

Would it be possible to think of a new convention here? I'm not against the convention but this feels like giving up early. Compatibility can always be preserved through aliases.

@Drup @dbuenzli you guys are pretty good with naming stuff.

Context: primed functions handle the query string as a (string * string) list while unprimed functions treat the query list as (string * string list) list).

For example:

val with_query : t -> (string * string list) list -> t
val with_query' : t -> (string * string) list -> t

@dsheets
Copy link
Member Author

dsheets commented May 18, 2015

Are you proposing the opposite convention of the initial proposal? I don't understand your comment about giving up early. If renames clash, aliases can't preserve compat...

I think the idea with the initial proposal is that most users don't care about the minutiae of query string syntax and interpretation and therefore the unprimed version should be the simpler type.

I'm not fond of having multiple similar functions differing only by ' tacked on to the end of their names.

Regardless of these issues, the current interface is not named consistently due to mistakes during the early development of the library. This should be fixed somehow.

@dbuenzli
Copy link

Context: primed functions handle the query string as a (string * string) list while unprimed functions treat the query list as (string * string list) list).

From the signature I vaguely understand the first, I don't understand the second one.

However what I really don't understand is that if I remember correctly RFC 3986 defines query as being a string. So it should rather be with_query : t -> string -> t and these derived functions should be provided in another helper Query module to devise query strings according to existing conventions.

@dsheets
Copy link
Member Author

dsheets commented May 18, 2015

Yes, query is opaque and these functions should be separated.

@rgrinberg
Copy link
Member

Are you proposing the opposite convention of the initial proposal? I don't understand your comment about giving up early. If renames clash, aliases can't preserve compat...

I'm saying that the primed/unprimed convention is possibly giving up early because the convention is not at all descriptive of the distinction it's describing. For example, a convention like adding _exn to raising functions is much more descriptive.

Unfortunately I can't think of any decent naming scheme.

@dsheets
Copy link
Member Author

dsheets commented May 18, 2015

Agreed. Most likely we should expose the internal structure of the library which has each of the syntactic components as nested modules. When this design is refined for 2.0, a suffix will probably become clear. Maybe _kv and _kvs?

@Drup
Copy link
Member

Drup commented May 18, 2015

I would tend to give the primary function the most specific type, t -> (string * string list) list -> t for example, and call the other function _raw. Not sure how it works with the functions that are with "a type slightly less precise but not completely raw", though.

@dbuenzli
Copy link

I would tend to give the primary function the most specific type, t -> (string * string list) list -> t for example, and call the other function _raw

I don't like this. If ocaml-uri is a general library you want to be able to work with the structures of the standard as they are named by it. Key/values queries and their syntax are outside the scope of the standard and as such should be provided somewhere else and/or under different names.

@dbuenzli
Copy link

For example these things have absolutely nothing to do in the core URI interface in my opinion.

@Drup
Copy link
Member

Drup commented May 18, 2015

My opinion is that the default interface should use types the most (we are not using a statically type language to encode everything in strings ...). Especially since it's more strict than the standard and that the more liberal functions are available.

If functions were more loose than the standard, I would agree, but it's not the case.

@dbuenzli
Copy link

Le lundi, 18 mai 2015 à 18:03, Gabriel Radanne a écrit :

If functions were more loose than the standard, I would agree, but it's not the case.

What you don't get here is that this is not part of the standard.

@dbuenzli
Copy link

Basically I would be much more clean to have the Uri module respect the names, semantics of URI components as defined by the standard and have a Query submodule that allows to generate queries as functions from richer types to string as defined by other standards (can't find a link to the key/value spec but I think it must be in the html standard, somewhere around forms). This would lead to code as follows:

Uri.(with_query u (Query.of_kvs kvs))
Uri.(Query.to_kvs (query u))

In this world there is no confusing priming/multiplication of entry points for URI components and we make a good distinction between what the standard is and how you can use it – for example the standard is absolutely not this which is confusing.

As a standard implementer it is also your duty to provide a good conceptual model and understanding of the standard through the API you provide. ocaml-uri seems to be failing on this point (which I as far as I read @dsheets, he acknowledges readily).

@Drup
Copy link
Member

Drup commented May 19, 2015

What you don't get here is that this is not part of the standard.

sight Yes, I get it, and I don't care, because ocaml-uri already does a bit more than the standard and that it's more convenient to the user this way.

@rgrinberg asked my opinion, I gave it. The most structured output first, the raw stuff in an auxiliary function, not the other way around. Using uri for now is annoying both because it's inconsistent and because it doesn't always give you the most structured representation (heh, path as raw string).

Outputing something very raw just to restructure it after while it's already structured inside Uri.t just sucks.

@dbuenzli
Copy link

sight Yes, I get it, and I don't care, because ocaml-uri already does a bit more than the standard and that it's more convenient to the user this way.

And annoying if that's not your use case, see #57. And frankly the fix, a verbatim_query function is both ugly and obscure (as would a raw_query be).

If you are reading code and a have a clear understanding of what an uri and its component are you can only wonder at what all these crap names are about. Concerns should be clearly separated with a good base Uri module that allows you to access uri components as named by the standard and submodules for further constructing and deconstructing these components.

Outputing something very raw just to restructure it after while it's already structured inside Uri.t just sucks.

Well the fact that it's apparently structured inside Uri is an implementation mistake to start with and is not relevant. There are a lot of cases where you want don't want to process the query part and as such the base module shouldn't even try to extract structure from it as it can only be a waste of cpu cycles in these cases.

@dbuenzli
Copy link

Having the names of the standard map to what they can actually represent is also much more useful for guiding people who didn't read the RFC or haven't a clear understanding of what an uri is. By not misleading them with wrong representation you minimize the chances that they do wrong assumptions about the way uris are supposed to be processed in general, which leads to better software in the end.

@rgrinberg
Copy link
Member

I agree with @dbuenzli on principle about having Uri fundamentally independent from any query handling. But that whole issue is orthogonal to the problem at hand here.

Even a hypothetical Query module will need some sort of convention to handle treating the query list as key value pairs vs. key list value pairs. So I don't see what's wrong with having a discussion here.

@dbuenzli
Copy link

Even a hypothetical Query module will need some sort of convention to handle treating the query list as key value pairs vs. key list value pairs. So I don't see what's wrong with having a discussion here.

Nothing wrong but the answers can be quite different, name wise. So what's the difference syntactically on an example I don't understand your wording ("key value pairs vs. key list value pairs") ?

@rgrinberg
Copy link
Member

I've explained in my first comment so that was just an attempt to wordit concisely:

Context: primed functions handle the query string as a (string * string) list while unprimed functions treat the query list as (string * string list) list).

For example:

val with_query : t -> (string * string list) list -> t
val with_query' : t -> (string * string) list -> t

In a hypothetical query module you could imagine it being

module Query : sig
  val update : t -> (string * string list) list -> t
  val update' : t -> (string * string) -> t
end

@dbuenzli
Copy link

while unprimed functions treat the query list as (string * string list) list)

Precisely I don't understand what this represents, how do you translate that to a query string ?

@rgrinberg
Copy link
Member

We can ask Uri:

# Uri.encoded_of_query ["foo",["bar";"baz"];"ocaml",["uri"]];;
- : bytes = "foo=bar,baz&ocaml=uri"

@dbuenzli
Copy link

"foo=bar,baz&ocaml=uri"

Is this standardized or just something that often happens ?

First gut reaction is that

t -> (string * string list) list -> t

should be at least

?sep:string -> t -> (string * string list) list -> t

@dbuenzli
Copy link

Something around multivalued comes to mind. Tentative:

Query.of_fields : (string * string) list -> string
Query.of_multi_fields : ?sep:string -> (string * string list) list -> string

@dbuenzli
Copy link

If deemed too long Query.of_multi_fields could be contracted to Query.of_mfields. Basically the interface introduces two notions, fields (a key to value map) consistently referred to in the API by field/fields and multivalued fields (a key to lists of values map) consistently referred to in the API by mfield/mfields.

@Drup
Copy link
Member

Drup commented May 20, 2015

Is this standardized or just something that often happens ?

http://www.w3.org/TR/html5/forms.html, grep ",", it's in various places.

@dbuenzli
Copy link

http://www.w3.org/TR/html5/forms.html, grep ",", it's in various places.

Excellent thanks ! This is precisely "When the multiple attribute is specified on the element", hence I think the multivalued idea is definitively on the right track.

W.r.t. to the sep optional parameter this should really be added, see e.g. the q query field in most search engines.

@dbuenzli
Copy link

W.r.t. to the sep optional parameter this should really be added, see e.g. the q query field in most search engines.

Ah but wait. You would certainly like to this on a per field basis. This suggests that maybe having fields/mfields functions is not a good idea in itself. Rather you would like to have good combinators to devise single or multivalued field values.

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

4 participants