Skip to content
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

Merged
merged 4 commits into from
Jun 16, 2022
Merged

Expand examples #8

merged 4 commits into from
Jun 16, 2022

Conversation

flyingsl0ths
Copy link
Contributor

No description provided.

@flyingsl0ths flyingsl0ths changed the title Resolves #3 Expand examples Jun 15, 2022
@flyingsl0ths flyingsl0ths changed the title Expand examples Resolves #3 Jun 15, 2022
@flyingsl0ths flyingsl0ths changed the title Resolves #3 Expand examples Jun 15, 2022

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) {
Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

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.


fn(temp_file);

std::fclose(temp_file);
Copy link
Member

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).

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';
Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Member

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) {
Copy link
Member

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?

Copy link
Contributor Author

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";
Copy link
Member

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.

@karnkaul
Copy link
Member

Close #3

@karnkaul karnkaul merged commit 66c4216 into main Jun 16, 2022
@karnkaul karnkaul deleted the Expand-example branch June 16, 2022 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants