-
-
Notifications
You must be signed in to change notification settings - Fork 9
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 support for multiple output files #156
Add support for multiple output files #156
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.
Noticed a few things, but generally I like how it's implemented!
lib/ronin/recon/cli/commands/run.rb
Outdated
options[:output_format].open(options[:output]) | ||
end | ||
output_files = outputs.filter_map do |output, output_format| | ||
output_format.open(output) if output && output_format |
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.
The output && output_format
test might not be necessary. Double checking OutputFormats.infer_from
, it appears to default to OutputFormats::TXT
, so we should always have an output format.
Ronin::Recon::OutputFormats.infer_from('foo.lol')
# => Ronin::Core::OutputFormats::TXT
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 removed only the output_format
just to be 100% sure there is always an output
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.
There should always be path
value if the --output
option is given, since the argument is required.
28a09b2
to
3de7e81
Compare
032be97
to
9e8f021
Compare
3de7e81
to
7446b66
Compare
7446b66
to
7d75441
Compare
#118
Loose idea, let me know what do you think @postmodern