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

Adapt CROW_WEBSOCKET_ROUTE to accept app reference #556

Merged
merged 4 commits into from
Dec 11, 2022

Conversation

soehrl
Copy link
Contributor

@soehrl soehrl commented Nov 2, 2022

Hello,

I noticed today that it is not possible to use the CROW_WEBSOCKET_ROUTE macro to define WebSocket routes in other functions. E.g., something like this:

void define_endpoints(crow::SimpleApp& app) {
    CROW_ROUTE(app, "/") ([]() {
        return "Hello, world!";
    });
    CROW_WEBSOCKET_ROUTE(app, "/ws")
      .onaccept([&](const crow::request&, void**) {
          return true;
      })
      .onopen([](crow::websocket::connection&) {})
      .onclose([](crow::websocket::connection&, std::string_view) {});
}

Resulted in the error:

error: no matching member function for call to 'websocket'
    CROW_WEBSOCKET_ROUTE(app, "/ws")
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/simon/Code/Crow/include/crow/app.h:33:98: note: expanded from macro 'CROW_WEBSOCKET_ROUTE'
#define CROW_WEBSOCKET_ROUTE(app, url) app.route<crow::black_magic::get_parameter_tag(url)>(url).websocket<decltype(app)>(&app)
                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
/Users/simon/Code/Crow/include/crow/routing.h:527:29: note: candidate template ignored: substitution failure [with App = crow::Crow<> &]: 'app' declared as a pointer to a reference of type 'crow::Crow<> &'
        WebSocketRule<App>& websocket(App* app)
                            ^            ~
1 error generated.

The issue was that decltype(app) then returned a reference. I fixed the problem by using std::remove_reference<decltype(app)>::type instead. I also added a test to assure that it is working.

@The-EDev The-EDev force-pushed the feature/fix-websocket-route-macro branch 2 times, most recently from 5d43b99 to f8f4216 Compare November 4, 2022 15:59
@The-EDev
Copy link
Member

The-EDev commented Nov 4, 2022

In function 'void define_endpoints(crow::SimpleApp&)':

/drone/src/tests/external_definition/main.cpp:13:54: error: 'std::string_view' has not been declared

   13 |       .onclose([](crow::websocket::connection&, std::string_view) {});

@soehrl
Copy link
Contributor Author

soehrl commented Nov 4, 2022

Didn't test it on an older compiler, should work now!

@crow-clang-format
Copy link

--- tests/external_definition/main.cpp	(before formatting)
+++ tests/external_definition/main.cpp	(after formatting)
@@ -1,8 +1,10 @@
 // Testing whether crow routes can be defined in an external function.
 #include "crow.h"
 
-void define_endpoints(crow::SimpleApp& app) {
-    CROW_ROUTE(app, "/") ([]() {
+void define_endpoints(crow::SimpleApp& app)
+{
+    CROW_ROUTE(app, "/")
+    ([]() {
         return "Hello, world!";
     });
     CROW_WEBSOCKET_ROUTE(app, "/ws")

@soehrl soehrl force-pushed the feature/fix-websocket-route-macro branch from 08599b0 to 1251009 Compare November 8, 2022 17:23
@soehrl soehrl force-pushed the feature/fix-websocket-route-macro branch from 1251009 to d0929ae Compare November 23, 2022 12:38
@The-EDev The-EDev merged commit cae4d3e into CrowCpp:master Dec 11, 2022
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.

3 participants