Arduino icon indicating copy to clipboard operation
Arduino copied to clipboard

Add pathArgs method in analogy to the args method of ESP8266WebServer

Open ahorn42 opened this issue 1 year ago • 4 comments

If you want to use path arguments (pathArg) together with UriRegex and optional catch groups in a regular expression you need to know how many path arguments have been found as the number might vary. Otherwise you might access indices that are not available depending on the number of matched optional catch groups.

This pull request implements the method pathArgs in analogy to the method args for normal server arguments.

Feel free tell me if something should be changed or if something is missing and also feel free to do changes your self to this PR if you want to.

I hope this features helps someone else as well :) - it makes path matching with regular expressions even more powerful.

ahorn42 avatar Mar 10 '24 12:03 ahorn42

(sry for delay)

I don't think Server params need to be renamed here? Server method can keep the new name, but Request object can simply use a different method (or Server can friend it) name to access the internal object. Would be much shorter diff, too, and would not break backwards compat in case anyone used the old names

mcspr avatar Mar 17 '24 17:03 mcspr

Just to clarify - you mean just the parameter name pathArgs in the signature of the canHandle method?

bool canHandle(const String &requestUri, std::vector<String> &pathArgs) override final {

It that case I would say it might possibly lead to slight confusion, but in general I would be fine in changing that back :)

ahorn42 avatar Mar 17 '24 20:03 ahorn42

I meant something like this

diff --git a/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h b/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h
index f7a95da0..9e6f299a 100644
--- a/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h
+++ b/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h
@@ -581,6 +581,13 @@ void ESP8266WebServerTemplate<ServerType>::_streamFileCore(const size_t fileSize
   send(200, contentType, emptyString);
 }

+template <typename ServerType>
+int ESP8266WebServerTemplate<ServerType>::pathArgs() const {
+  if (_currentHandler != nullptr)
+    return _currentHandler->pathArgsSize();
+  return 0;
+}
+
 template <typename ServerType>
 const String& ESP8266WebServerTemplate<ServerType>::pathArg(unsigned int i) const {
   if (_currentHandler != nullptr)
diff --git a/libraries/ESP8266WebServer/src/ESP8266WebServer.h b/libraries/ESP8266WebServer/src/ESP8266WebServer.h
index 397132f1..d7e6f9b5 100644
--- a/libraries/ESP8266WebServer/src/ESP8266WebServer.h
+++ b/libraries/ESP8266WebServer/src/ESP8266WebServer.h
@@ -136,6 +136,7 @@ public:
   ServerType &getServer() { return _server; }

   const String& pathArg(unsigned int i) const; // get request path argument by number
+  int pathArgs() const;
   const String& arg(const String& name) const;    // get request argument value by name
   const String& arg(int i) const;          // get request argument value by number
   const String& argName(int i) const;      // get request argument name by number
diff --git a/libraries/ESP8266WebServer/src/detail/RequestHandler.h b/libraries/ESP8266WebServer/src/detail/RequestHandler.h
index 4195f0ff..aacbb8b1 100644
--- a/libraries/ESP8266WebServer/src/detail/RequestHandler.h
+++ b/libraries/ESP8266WebServer/src/detail/RequestHandler.h
@@ -27,10 +27,14 @@ protected:
     std::vector<String> pathArgs;

 public:
-    const String& pathArg(unsigned int i) {
+    const String& pathArg(unsigned int i) const {
         assert(i < pathArgs.size());
         return pathArgs[i];
     }
+
+    int pathArgsSize() const {
+        return pathArgs.size();
+    }
 };

 } // namespace

Since it is not shadowing anything while in the Server class context

edit: Also, check out CI logs down below. It fails when trying to build our example sketches

mcspr avatar Mar 17 '24 20:03 mcspr

Ah, okay - I think I now got what you mean - I will have look at it, maybe not today anymore, but I try to have a look in the next days!

ahorn42 avatar Mar 17 '24 20:03 ahorn42