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

Add a ParamNames method on Context #355

Merged
merged 1 commit into from Feb 5, 2016

Conversation

Projects
None yet
2 participants
@arkan

arkan commented Feb 4, 2016

Hello,

Currently there is no way to get all matched parameters from the Context. While this fully makes senses in the context of a classic webapp where knowledge of which parameters are passed is implied, this doesn't allow to write a dynamic handler that does for example, some forwarding on another transport layer (which requires to aggregate parameter names and values).

Given the methods that are currently available we'd have to rematch the route to infer the parameters but that defeats the purpose of using echo as it's made for speed which is exactly what we're looking for here.

A single method returning pnames appears to be a fair tradeoff to fix our use case without changing too much on your side nor messing with your API design.

Thanks

@vishr

This comment has been minimized.

Member

vishr commented Feb 4, 2016

Thanks for reporting. There was a related issue #103, where I mentioned to expose similar APIs. Do you think exposing c.ParamValues() too is useful too (Just being lazy to think about it). On a separate note, if you are using Echo in your company/projects consider getting it listed #295

@arkan

This comment has been minimized.

arkan commented Feb 4, 2016

c.ParamValues could be added but isn't required per se, which could very well be a good reason to not add it.

In our case we're building a map and the following code is pretty straightforward:

params := map[string]string{}
for _, key := range c.ParamNames() {
    params[key] = c.Param(key)
}

About #295 we'll happily add us there :simple_smile:

@vishr

This comment has been minimized.

Member

vishr commented Feb 4, 2016

Can we have a test case?

@arkan

This comment has been minimized.

arkan commented Feb 5, 2016

🆙

vishr added a commit that referenced this pull request Feb 5, 2016

Merge pull request #355 from heetch/feature/add-param-names
Add a ParamNames method on Context

@vishr vishr merged commit 9f3aaa8 into labstack:master Feb 5, 2016

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 92.875%
Details
@vishr

This comment has been minimized.

Member

vishr commented Feb 5, 2016

@arkan Where are you using Echo?

@arkan arkan deleted the heetch:feature/add-param-names branch Feb 5, 2016

@vishr vishr referenced this pull request Feb 10, 2016

Closed

Echo v2 proposal #294

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