-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(forge): flatten #506
feat(forge): flatten #506
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.
lg so far,
some parts I already refactored on the config branch, but +1 on merging this before and I'll rebase #297
match self.output { | ||
Some(output) => { | ||
std::fs::create_dir_all(&output.parent().unwrap())?; | ||
std::fs::write(&output, flattened)?; | ||
println!("Flattened file written at {}", output.display()); | ||
} | ||
None => println!("{}", flattened), | ||
}; |
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.
writing to a file is perhaps redundant because you could just do
forge flatten Contract.sol > flattened.sol
either way you'd need to provide the output path
But I have no strong feelings about this argument
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.
correct and i've got no string feelings either, happy to change if needed
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.
leaning towards removing the output option, @gakonst
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 think let's keep cuz I like "intuitively" that you can just consume it this way without knowing OS level things like pipes/redirects, even if they are really basic
Addresses #312