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

add support for Command filter #168

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

add support for Command filter #168

wants to merge 11 commits into from

Conversation

gkorland
Copy link
Contributor

No description provided.

src/commandfilter/mod.rs Outdated Show resolved Hide resolved
src/commandfilter/mod.rs Outdated Show resolved Hide resolved
src/commandfilter/mod.rs Show resolved Hide resolved
src/commandfilter/mod.rs Show resolved Hide resolved
@gkorland gkorland requested a review from oshadmi July 19, 2021 12:34
examples/command_filter.rs Show resolved Hide resolved
unsafe { raw::RedisModule_CommandFilterArgsCount.unwrap()(self.ctx) as usize }
}

pub fn args_get(&self, pos: usize) -> Option<RedisString> {
Copy link
Contributor

Choose a reason for hiding this comment

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

The naming is inconsistent with the Redis APIs: "args" vs "arg". Although the APIs are themselves a bit inconsistently named, let's at least not add even more to the confusion and use the same names here.

In any case, it's probably best to wrap this low-level API wrapper with a higher level idiomatic Rust API (e.g. providing an iterator over the args), but that's not urgent.

status
}

pub fn args_replace(&self, pos: usize, arg: RedisString) -> Status {
Copy link
Contributor

Choose a reason for hiding this comment

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

The insert and replace functions are mostly identical; it may make sense to unify the common code (freeing the strings etc.)

@@ -82,6 +82,31 @@ macro_rules! redis_event_handler {
}};
}

#[cfg(feature = "experimental-api")]
#[macro_export]
macro_rules! redis_commnad_filter {
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo:

Suggested change
macro_rules! redis_commnad_filter {
macro_rules! redis_command_filter {

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.

3 participants