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

Add a catch-all handler 2nd version #122

Merged
merged 5 commits into from
Apr 5, 2021
Merged

Conversation

Tibbel
Copy link
Contributor

@Tibbel Tibbel commented Apr 5, 2021

Here is the modified catchall-function
example added
the code moved to routing.h
only the setter remained in app.h
the integer return is removed
i personally don't need the default parameters:
without crow::response the catchall can take no effect
and without crow::request you know nothing about the request
i only imagine one case, this is to set custom error pages.
so i don't thing default parameters have any big benefit here.
"#ifdef CROW_CATCHALL" was only for me, to find my own modifications feel free to replace/rename it

Tibbel added 2 commits April 5, 2021 09:44
Here is the modified catchall-function
example added
the code moved to routing.h
only the setter remained in app.h
the integer return is removed
i personally don't need the default parameters:
without crow::response the catchall can take no effect
and without crow::request you know nothing about the request
i only imagine one case, this is to set custom error pages.
so i don't thing default parameters have any big benefit here.
"#ifdef CROW_CATCHALL" was only for me, to find my own modifications feel free to replace it
Add a catch-all handler 2nd version
@Tibbel
Copy link
Contributor Author

Tibbel commented Apr 5, 2021

It would be a lot nicer if CROW_CATCHALL was like CROW_ROUTE as in you just use the macro to create a catchall route.

for me macros are more c'ish and not really c++ i use it only for workarounds, but here is one:
#define CROW_CATCHALL(a, f) a.catchall_handler( f );

@Tibbel Tibbel closed this Apr 5, 2021
@Tibbel Tibbel reopened this Apr 5, 2021
@The-EDev
Copy link
Member

The-EDev commented Apr 5, 2021

HI, Thanks for taking the time to change the original PR.

I've noticed there are large differences between this PR and how Crow is written, so instead of merging into master I'll merge it into a feature branch and work from there to make sure everything is uniform.

I'll also open an issue to keep track of any discussion.

Thanks again for your work and for bringing this feature up.

@The-EDev The-EDev changed the base branch from master to catch_all April 5, 2021 11:16
@The-EDev The-EDev merged commit 0d7c8ed into CrowCpp:catch_all Apr 5, 2021
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