-
Notifications
You must be signed in to change notification settings - Fork 301
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
Refactor redactor into replacer #2277
Conversation
13a2448
to
6ea9be0
Compare
if _, err := r.dst.Write(r.subst); err != nil { | ||
// Now handle the match itself. | ||
// Call the replacement callback to get a replacement. | ||
if repl := r.replacement(r.buf[match.from:match.to]); len(repl) > 0 { |
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.
is "replace the thing matched with an empty string" (ie delete the matched string) a valid use case? i can see the semantics there being a little confusing if the user tried to do that
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.
That should be supported - replacement
should return nil
. The if
on this line is just to skip calling r.dst.Write
with nil or empty.
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.
sweet as
I'm going to add some more tests, since there is 0 coverage in most of Edit: Done. |
6ea9be0
to
2b08f0d
Compare
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.
Nice. I think I know how to use this now.
This changes
redactor
intoreplacer
. A summary:ValuesToRedact
, andVarsToRedact
are moved into a new package,redact
Redactor
and necessary friends are renamed and moved intoreplacer
.pipeline_upload.go
is changed to use aReplacer
to search through the JSON in a streaming manner.flushUpTo
is changed to call a callback when processing matches instead of using a fixed[]byte
.flushUpTo
also no longer eagerly flushes into the middle of matches.