-
Notifications
You must be signed in to change notification settings - Fork 1
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
Plain text capture #38
Conversation
@aggroskater, would this work for you? |
That would certainly work! Looks nice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
doc | ||
else if not _.isFunction(doc.html) and typeof doc is 'string' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing this on staging, I thought it didn't work at first, because the dummy service I was hitting returned HTML, causing this to be skipped. Why skip this for HTML, when there's no other handling for that format anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My original code just placed the entire raw body in a "body" property of event
, and I didn't want to include gobs of HTML in that property. Now that we're using regex capture groups, I suppose we can remove that condition. Granted, one should never parse HTML with regex, but otherwise it's better than not having any option at all (and I suspect most would only be looking for specific text anyway and not trying to do anything crazy like build an HTML parser out of regex :D).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Thanks. After discussing it more internally, we agreed: most uses would be searching for specific text snippets somewhere in the HTML doc. We decided to remove this restriction, it should come pretty soon.
Note, though, that this version (2.9.0
) is now live in staging & production. 👍
This PR was based off of #37.
The change adds a feature that captures part of a text/plain response body using a regular expression. For example...
Response body:
Mapping:
And the integration will append: