pat: support generating URLs from a Pattern

[metcalf]

Adds support for generating paths from a Pattern and a mapping from named matches to string values. This is useful in writing API clients as it allows for constructing URLs that exactly match the routing table with semantically meaningful arguments.

[metcalf]

I'm completely unattached to this method name.

[zenazn]

I might just call it Path (or URL and return a url.URL). Seems a bit simpler.

We could also try to make it round-trip with Match and return a full-blown http.Request but that seems like it's a bit much.

[zenazn]

One other thing: this takes map[pattern.Variable]string, while much of the rest of the pattern ecosystem produces and consumes map[pattern.Variable]interface{}. I know it's kind of ugly (and if I end up regretting one thing from this redesign I feel like it'll probably be that type), but we should probably accept that type here as well (and type assertion our way to victory)

[zenazn]

I'd probably prefer looping through pats and literals here: it's a little bit ugly, but I suspect it'll be more efficient, and the error handling becomes a bit nicer since we don't need to deal with escaping the callback.

[zenazn]

Could you also rebase this on master so we pick up Travis tests?

[metcalf]

@zenazn: Responded to comments and rebased on master to pick up tests.

The only comment I disagree with is changing the interface to accept a map[pattern.Variable]interface{}. The only place I see map[pattern.Variable]interface{} is in using AllVariables. I think I agree with that choice -- a custom pattern may want to implement e.g. coercion integer types and can implement typed getters (as you do with Pattern.Param).

In this case, it doesn't make much semantic sense to consider the output of Context.Value(AllVariables) as the input to generate a new URL since those variables are generated from the URL in the first place. Further, Pattern.Param implies a mapping from pattern.Variable to string for this package. I think this is a case where pat represents a specialization that is more clearly rooted in strings.

[zenazn]

you normally pass around *url.URL

[zenazn]

The biggest reason to use the AllVariables type is not because it makes this package better (it doesn't: it makes it worse), but because it allows you to write abstractions across ~all pattern types (or at least the ones that opt-in to this mechanism).

So to be fair, I don't know that this even a thing I want, but that's the thing at stake here. I agree with you that it makes little sense for this package in particular, but a stronger goal of mine is to build a good ecosystem of stable and flexible types.

[metcalf]

Thinking about it more, I think AllVariables should probably be a url.Values or some other string-string mapping.

The counter point would be providing support for things like "converters" in Flask (concretely, supporting /foo/<int:id>/bar/<bool:flag>). If I were implementing something like that in my pattern matcher, I'd probably expose it using typed getters like IntParam that can handle the conversion from strings to the desired type. If I didn't want to do that conversion every time the getter is called, I could cache my own conversions in the request context but I don't see a need to share that between patterns in a standard way.

Realistically I don't think I'd implement the "converter" semantic this way in Go at all. Instead I'd compose goji with another converter like Gorilla's schema. Using a common type like url.Values would enhance the composability of the pattern ecosystem with little cost to its power.

[metcalf]

(oh... and I'm a little braindead getting over the flu so please tell me if that last comment was rambling and incomprehensible)