Skip to content

updates to use context from the request for params instead of passing…#41

Open
husobee wants to merge 1 commit intomasterfrom
context
Open

updates to use context from the request for params instead of passing…#41
husobee wants to merge 1 commit intomasterfrom
context

Conversation

@husobee
Copy link
Owner

@husobee husobee commented Aug 16, 2016

… them in the url

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 99.579% when pulling 1f4e24f on context into 8a7d81c on master.

@josephspurrier
Copy link
Contributor

Awesome!

} else {
r.URL.RawQuery += q
paramNames := []string{name}
if v, ok := r.Context().Value("vestigo_param_names").([]string); ok {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as below.

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 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as below.

}
}
ctx := context.WithValue(r.Context(), "vestigo_"+name, value)
ctx = context.WithValue(ctx, "vestigo_param_names", paramNames)
Copy link

@nhooyr nhooyr Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@tw1nk
Copy link

tw1nk commented May 16, 2017

When will this be merged to master?

@husobee
Copy link
Owner Author

husobee commented May 16, 2017 via email

@slotheroo
Copy link

+1 for interest in seeing this merged

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants