Conversation
|
One note: the order of the paths are key! Given When calling In order to make this work: |
Could you add this explanation in your example ? |
devyte
left a comment
There was a problem hiding this comment.
Some questions at this point before I do a full review.
The order no longer matters. |
d-a-v
left a comment
There was a problem hiding this comment.
There is a trailing space on line 602 in the example.
I just realized that there would be a lot more constref to modify in these classes.
I'm OK to merge this now and it could be revisited later as part of a cleaning process (aka our ram is not gruyère)
|
@Bmooij I've been looking at this from different angles and... the {} syntax bothers me, it's just not standard. Extending it would require extending the custom parser code, which again would not be standard, so I'd prefer to avoid it. |
devyte
left a comment
There was a problem hiding this comment.
Consider using glob matching instead of custom syntax/code.
|
@devyte I'm open for every thing. I'm not familiair with glob. Can you show an example how the path would look like? |
|
/users/*/devices glob is actually used for filenames in a dir. I think the underlying function to use is fnmatch (fnmatch.h). It returns zero or non-zero. Printout is ret=0, which means match. |
|
This requires further discussion, so pushing back. |
|
After using glob patterns, my personal preference goes to regex. usage: note: |
|
This has a merge conflict, and needs further consideration to evaluate against the alternatives. Pushing back. |
|
I forgot this pull request. Tomorrow I will fix the merge conflicts and implement the A problem that I encountered from a similar pull request, was that regex is a large include (100k). |
|
@Bmooij #5467 I already implemented glob. All 3 alternatives are in PRs. onRegEx()
onGlob()
onDontKnowNameOfThisOne()With this, the regex bin increase should happen only if it is actually used, and yet all three alternatives are available. |
|
@devyte What do you think of the following solution: server.on("a/static/uri", callbackfn); // this will automatically transform to Uri("a/static/uri")
server.on(Uri("an/other/static/uri"), callbackfn);
server.on(UriRegex("^\\/users\\/([0-9]+)\\/devices\\/([0-9]+)$"), callbackfn);
server.on(UriGlob("/users/*/devices/*"), callbackfn);
server.on(UriBracket("/users/{}/devices/{}"), callbackfn); |
|
That is a very good idea!
|
|
@Bmooij could you please pick up the three implementations from the PRs and make a new one with your idea? That new one would then supersede the three. |
Path arguments may not contain the value '/' Updated the example
fix trailing space
e3ad036 to
879e076
Compare
|
There is an issue when I want to include lib\ESP8266WebServer\src/detail/RequestHandlersImpl.h:25:13: error: 'regex' is not a member of 'std' |
|
Closing in favor of #6696. |
Adding path arguments to WebServer. (In respond to #2019)
Ex.
I don't know if this is the correct approach, feel free to give comments.