updates to use context from the request for params instead of passing… #41
Conversation
|
Awesome! |
| } else { | ||
| r.URL.RawQuery += q | ||
| paramNames := []string{name} | ||
| if v, ok := r.Context().Value("vestigo_param_names").([]string); ok { |
There was a problem hiding this comment.
Do not use a plain string type like that. See https://blog.golang.org/context#TOC_3.2.
| if strings.HasPrefix(k, ":") { | ||
| names = append(names, k) | ||
| } | ||
| if v, ok := r.Context().Value("vestigo_param_names").([]string); ok { |
| @@ -29,30 +30,34 @@ var AllowTrace = false | |||
|
|
|||
| // Param - Get a url parameter by name | |||
| func Param(r *http.Request, name string) string { | |||
| return r.URL.Query().Get(":" + name) | |||
| // use the request context | |||
| if v, ok := r.Context().Value("vestigo_" + name).(string); ok { | |||
| } | ||
| ctx := context.WithValue(r.Context(), "vestigo_"+name, value) | ||
| ctx = context.WithValue(ctx, "vestigo_param_names", paramNames) |
There was a problem hiding this comment.
I think a better solution would be to store a map[string]string in the context with context.WithValue and then use that to return the parameter, instead of using context.WithValue for every single parameter. This would also prevent multiple lists from being stored. Above, for every URL parameter, a new context with the slice paramNames and key "vestigo_param_names" is linked with the others. It would also let you have a single package level key to access the map and prevent collisions. See https://github.com/pressly/chi/blob/master/context.go#L50 for an example.
|
When will this be merged to master? |
|
This has been on Ice for a bit. I will take a look at this this week. I want to make sure it doesn't break anything before merging.
…On May 16, 2017 10:58:19 AM EDT, Calle Gustafsson ***@***.***> wrote:
When will this be merged to master?
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#41 (comment)
|
|
+1 for interest in seeing this merged |
… them in the url