-
Notifications
You must be signed in to change notification settings - Fork 387
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
feat(package): /p/demo/mux
support query strings
#2594
base: master
Are you sure you want to change the base?
feat(package): /p/demo/mux
support query strings
#2594
Conversation
/p/demo/mux
support query strings/p/demo/mux
support query strings
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
I am thinking about if we want to use avl tree to store the params instead of using map |
@moul @zivkovicmilos can you have a look into this? Thanks. |
*/ | ||
func (r *Request) Query() UrlQuery { | ||
urlQueries := UrlQuery{} | ||
reqParts := strings.Split(r.Path, "/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think that you're not supporting having a slash in one of the query parameters:
gno.land/p/demo/foo?a=b&c=d/e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I saw your TODO
in current request parser. I will try to update for this /a/b/c/d/...
case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're just adding new helpers, but the underlying existing library won't work with the changes, please add more unit tests for the existing helpers with query strings.
like hello/{name}
with hello/Alice?a=b
-> currently name equals Alice?a=b
instead of Alice
.
@moul |
8d78ad4
to
48a0eaa
Compare
What is the state of this PR? |
@leohhhn |
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the descriptionThis PR aims to support
mux
package for querying with strings. (relates #2585 )Usage: