-
Notifications
You must be signed in to change notification settings - Fork 467
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
small improvements to existing FileParsers #107
Conversation
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, great refactoring of the lambs
def __init__(self, file): | ||
self.parser = configparser.ConfigParser() | ||
self.parser.optionxform = str | ||
self.parser.read_file(file) | ||
|
||
# Hacky way to keep track of line location | ||
file.seek(0) | ||
self.lines = list(map(lambda x: x.strip(), file.readlines())) |
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.
++
# as more language specific file parsers are implemented. | ||
# discussion: https://github.com/Yelp/detect-secrets/pull/105 | ||
WHITELIST_REGEX = { | ||
'yaml': WHITELIST_REGEXES[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.
Super Nit: Maybe add to the regex comment # e.g. python
-> # e.g. python or yaml
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.
I don't think that'll be necessary if a python file parser is implemented. Then we can 'py': WHITELIST_REGEXES[0]
. More language/specification specific file parsers is what I'm pushing for, or at least suggesting, architecturally. See #105.
IniFileParser
; things needed in smaller scopes were lazily created and constants were moved to object-level scope.YamlFileParser
: small follow up to add whitelist directive regexes to match against inline comment syntaxes in more languages #105; only the whitelist directive regex for YAML inline comment syntax should be used.