Issue#3762 Adding Regex MatchAll feature - #6234
Conversation
|
@BadBuddha thanks for the PR, this looks good! We'll want to add some polyglot tests for this. See Once that's done we'll need to implement the new |
| @@ -2,2420 +2,2433 @@ | |||
| // hash: "186fb20ea9911710e910acfab0f4f221d59d7c04" | |||
There was a problem hiding this comment.
I don't see the documentation of matchAll in that branch. Did you edit this file manually?
There was a problem hiding this comment.
I'll checkout reql_docs.js, I think there was issue with atom editor. Sorry should have seen this.
| *last_is_dot = false; | ||
| *last_should_r_wrap = false; | ||
| return make_optional(var.arg(0)); | ||
| return boost::make_optional(var.arg(0)); |
There was a problem hiding this comment.
What happened here? This change seems unrelated.
There was a problem hiding this comment.
I don't remember changing this, will 'checkout' this file too
|
The code looks good to me. However the commit includes a lot of unrelated changes to With that fixed, I think it could be merged. However it would still be missing tests, and needs to be added to the other drivers. |
|
I'll add tests and add this to the other drivers. I should just search for all the places match is being used as matching all and replace it with match_all ? |
|
I would add separate tests for For the python driver it should be added in two places in https://github.com/rethinkdb/rethinkdb/blob/next/drivers/python/rethinkdb/ast.py For ruby and java it should be similar. |
|
Pointing at v2.4.x in anticipation of making that the new main branch of this repo. I intend to revisit this after v2.4.2 is released. |
|
FTR: |
Description
<Please fill in the description of your pull request here. Thank you for your contribution!>