Skip to content
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

Parse and populate route parameters #32

Merged
merged 4 commits into from
Jun 20, 2022
Merged

Parse and populate route parameters #32

merged 4 commits into from
Jun 20, 2022

Conversation

pictographer
Copy link
Contributor

No description provided.

@pictographer pictographer requested a review from jbearer June 18, 2022 00:36
@pictographer pictographer marked this pull request as draft June 18, 2022 00:42
@pictographer pictographer marked this pull request as ready for review June 18, 2022 01:07
Comment on lines +204 to +205
// TODO Should the map key and name be different? If
// not, then RequestParam::name is redundant.
Copy link
Member

Choose a reason for hiding this comment

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

You're right, RequestParam::name is probably redundant

RequestParam {
name: seg.to_string(),
param_type: ptype,
// TODO How should we encode optioanl params?
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking for now we would just check if the parameter appears in every pattern for the route. If so, it is "required".

I think later on, we should separate parameter declarations from route patterns, something like:

":my_param" = { type = "TaggedBase64", doc = "A parameter", optional = true }

and then allow parameters to be passed by URL segment, URL param, or request body.

@pictographer pictographer merged commit 112b7a2 into main Jun 20, 2022
@Ancient123 Ancient123 deleted the corbett-route-new branch November 29, 2022 15:59
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.

2 participants