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

Adding support for scan and flushdb commands. #38

Merged
merged 2 commits into from
Apr 11, 2019
Merged

Adding support for scan and flushdb commands. #38

merged 2 commits into from
Apr 11, 2019

Conversation

davidkus
Copy link

Looks like there is only 1 database, so I've made the flushdb command perform the same exact operation as flushall.

Resolves #33

Copy link
Owner

@fppt fppt left a comment

Choose a reason for hiding this comment

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

Thank you so much for the contribution! I only really have one nitpick. If you fix it awesome, if not I will merge and release sometime this week or early next week.

return Response.array(response);
}

private static Slice extractParameter(List<Slice> params, String name) {
Copy link
Owner

Choose a reason for hiding this comment

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

So I am being a bit nitpicky but I would say rather return an Optional<Slice> here. This will clean up your consumer calls, i.e. rather than:

Slice matchSlice = extractParameter(params(), MATCH);
String match = matchSlice != null ? matchSlice.toString() : "*";

to

String match = extractParameter(params(), MATCH).orElse("*")

Copy link
Author

Choose a reason for hiding this comment

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

Done! It's a bit more involved as extractParameter returns Slice and not String.

@@ -87,4 +87,32 @@ public static int convertToInteger(String value){
throw new WrongValueTypeException("ERR bit offset is not an integer or out of range");
}
}

public static String createRegexFromGlob(String glob)
Copy link
Owner

Choose a reason for hiding this comment

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

Definitely belongs here!

@fppt fppt merged commit f0aa3d5 into fppt:master Apr 11, 2019
@fppt
Copy link
Owner

fppt commented Apr 11, 2019

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.

2 participants