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
Fix 'ignore' for keys containing key delimiter #209
Conversation
Pull Request Test Coverage Report for Build 1044509237
💛 - Coveralls |
Pull Request Test Coverage Report for Build 1045049814
💛 - Coveralls |
lib/utils.js
Outdated
*/ | ||
function deleteLogProperty (log, property) { | ||
const props = property.split('.') | ||
const props = property.match(/([^\\.]|\\\.)+/g).map((e) => e.split('\\.').join('.')) |
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.
how expensive is this operation?
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.
https://jsbench.me/tqkrafwdkf/1
I just performed some benchmarks, this implementation is ~81% slower compared to just a split.
There might be quite a hit on performance with this change, however, this can be improved if we split the key using a loop. Benchmark shows that it is ~36% slower compared to just a split.
Will be pushing the improved version shortly for review.
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.
Thank you!
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
Thanks very much for this PR! |
This PR fixes the issue reported by @acidoxee in #132 (comment) just before the issue was closed.
This commit attempts to overcome the limitation by allowing the user to escape the dot character used as a delimiter.
The method used is to split the key by the dot delimiter ignoring any escaped dots, followed by replacing all occurrences of the escaped dots. Alternatively, it is possible to use a negative lookbehind regex for the split, but might not be supported on all browsers.
Thanks!