-
Notifications
You must be signed in to change notification settings - Fork 0
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
Expand examples #8
Conversation
example/forest-example.cpp
Outdated
|
||
constexpr auto foo = forest::literal<64>("<rgb=500><b><i>hello</b></i></rgb> world"); | ||
|
||
void with_temp_file(std::function<void(std::FILE*)> const fn) { |
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.
Use a template parameter typename F
instead of std::function
? It's quite heavy and mostly unnecessary here.
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.
Should I add an std::is_invocable_v
check as well?
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.
Up to you! Since this is example "client side" code, it's not very important; different story if it were a library function.
example/forest-example.cpp
Outdated
|
||
fn(temp_file); | ||
|
||
std::fclose(temp_file); |
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.
This is currently a bit unsafe: eg, if fn
throws, the file will remain locked and undeleted. I'd suggest wrapping the "close" logic into a lambda, and temp_file
into std::unique_ptr<std::FILE*, decltype(close_file)>
. That way the file will always be closed on destruction of the unique ptr / the function returning (even if it unwinds due to a throw).
example/forest-example.cpp
Outdated
int main() { | ||
std::cout << foo << '\n'; | ||
|
||
std::cout << forest::literal<71>("I <strike>copy</strike> <b>study</b> code from <u>stackoverflow.com</u>") << '\n'; |
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.
Might want to reduce the number of magic numbers a bit; eg use forest::length()
for one or two?
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 does it differ from say a call to std::string_view::size
?
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.
forest::length
will compute the size of the string after all replacements have been done. Ideally the library would call this first, figure out the required size, and then return a literal of that size, all at compile time. But getting that to work is largely impossible in C++17. Still, something like this is possible, although not great:
constexpr auto text = "<b>hi</b>";
constexpr auto output = forest::literal<forest::length(text)>(text);
forest::format_to(std::back_inserter(ft), "<b><rgb=155>Hello from the string!</rgb></b>"); | ||
std::cout << ft << '\n'; | ||
|
||
with_temp_file([](std::FILE* const tf) { |
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.
Nice pattern! Inspired from Python?
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.
Haskell 😎
|
||
constexpr auto foo = forest::literal<64>("<rgb=500><b><i>hello</b></i></rgb> world"); | ||
|
||
void with_temp_file(std::function<void(std::FILE*)> const fn) { | ||
template <typename F> | ||
void with_temp_file(F const fn) { | ||
constexpr auto tf_name = "temp.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.
Suggestion: make this static
to auto capture it.
Close #3 |
No description provided.