Skip to content

Conversation

@alexanderchan-scality
Copy link
Contributor

Original code will evaluate regex in /pattern/ syntax incorrectly
Adds parser to have MD search recognize if a regex is in /pattern/ syntax or
a simple string

rahulreddy
rahulreddy previously approved these changes Apr 27, 2018
dora-korpar
dora-korpar previously approved these changes Apr 27, 2018
*/
function parseLikeExpression(regex) {
const split = regex.split('/');
if (split.length < 3) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we include the case where split[0] != '' here, i.e. checking if regex starts with a slash?

Copy link
Contributor Author

@alexanderchan-scality alexanderchan-scality Apr 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. you're correct, I should be checking if the leading character is a slash. Will fix this.

@ironman-machine ironman-machine dismissed stale reviews from rahulreddy and dora-korpar April 27, 2018 22:28

Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

Original code will evaluate regex in `/pattern/` syntax incorrectly
Adds parser to have MD search recognize if a regex is in `/pattern/` syntax or
a simple string
@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@rahulreddy rahulreddy merged commit a79b5de into z/1.0 Apr 30, 2018
@rahulreddy rahulreddy deleted the bf/ZENKO-250-regexEval branch April 30, 2018 16:59
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.

6 participants