-
Notifications
You must be signed in to change notification settings - Fork 312
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
tool: Use @file syntax to loading data from a file #647
Conversation
Codecov Report
@@ Coverage Diff @@
## master #647 +/- ##
==========================================
- Coverage 92.86% 92.84% -0.02%
==========================================
Files 23 23
Lines 3546 3552 +6
Branches 375 376 +1
==========================================
+ Hits 3293 3298 +5
Misses 144 144
- Partials 109 110 +1 |
For --input and code arguments allow loading data from a file by preceding the file path with @ character. E.g. `--input @sha1.in`.
func_ = [](const std::string& str) -> std::string { | ||
if (!str.empty() && str[0] == '@') | ||
return CLI::ExistingFile(str.substr(1)); |
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.
Shouldn't this try to load & verify the content?
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.
It could but it feels too much. I don't know any CLI library doing so. And you would need to load the file twice.
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.
Since the files are usually small, the overhead may be worth it here for easy-of-use (and not relying on exceptions).
tools/evmc/main.cpp
Outdated
return str; | ||
if (str[0] == '@') // The argument is file path. | ||
{ | ||
std::ifstream file{str.c_str() + 1}; |
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.
Why use c_str() + 1
here and substr(1)
below?
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.
Here the constructor of const char*
is available.
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 actually changed this to add more info to the error.
Change the type of code and input parameters of tooling::run() from hex-encoded string to bytes. The hex decoding must happen outside. This moves the hex decoding errors out of the function concern.
For
--input
andcode
arguments allow loading data from a file by preceding the file path with @ character. E.g.--input @sha1.in
.Change the type of code and input parameters of
tooling::run()
from hex-encoded string to bytes. The hex decoding must happen outside. This moves the hex decoding errors out of the function concern.