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 regexp matching support to EventFormattingAgent. #214

Merged
merged 4 commits into from
Apr 15, 2014
Merged

Add regexp matching support to EventFormattingAgent. #214

merged 4 commits into from
Apr 15, 2014

Conversation

knu
Copy link
Member

@knu knu commented Apr 14, 2014

A new setting key matchers is added to define regular expression
matching against contents of events and expand the match data for use
in instructions setting.

A new setting key `matchers` is added to define regular expression
matching against contents of events and expand the match data for use
in `instructions` setting.
def validate_options
errors.add(:base, "instructions, mode, skip_agent, and skip_created_at all need to be present.") unless options['instructions'].present? and options['mode'].present? and options['skip_agent'].present? and options['skip_created_at'].present?

case matchers = options['matchers']
Copy link
Member

Choose a reason for hiding this comment

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

Because this is long, I'd suggest extracting it into a secondary private method, perhaps called validate_matchers, and calling it here.

Copy link
Member

Choose a reason for hiding this comment

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

It would also be nice if empty string ('') was treated like nil, since users often clear values out to remove them.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, the method originally had just one line, so I just didn't bother making a separate method.

@cantino
Copy link
Member

cantino commented Apr 15, 2014

This is a really useful addition, thank you @knu!

@knu
Copy link
Member Author

knu commented Apr 15, 2014

Updated!

it "should validate the contents of matchers" do
@checker.options[:matchers] = [
{}
]
Copy link
Member

Choose a reason for hiding this comment

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

Missing a should statement here I think.

@cantino
Copy link
Member

cantino commented Apr 15, 2014

Looks great, thanks @knu. At the moment, I don't think :to can be a JSONPath, but that's fine for now. A future improvement :)

@knu
Copy link
Member Author

knu commented Apr 15, 2014

Yeah. I'm not so familiar with JSONPath I don't even know if there is a writer API, but I'd like to support that if possible.

cantino added a commit that referenced this pull request Apr 15, 2014
Add regexp matching support to EventFormattingAgent.
@cantino cantino merged commit 623e6db into huginn:master Apr 15, 2014
@cantino
Copy link
Member

cantino commented Apr 15, 2014

Great addition @knu, thanks!

@knu knu deleted the event_formatting_agent-regexp branch August 13, 2014 14:42
DataMinerUK pushed a commit to DataMinerUK/huginn that referenced this pull request Oct 6, 2014
Add regexp matching support to EventFormattingAgent.
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