marzer / tomlplusplus

Header-only TOML config file parser and serializer for C++17.
https://marzer.github.io/tomlplusplus/
MIT License
1.53k stars 146 forks source link

Get toml/yaml/json string without using streams #216

Closed mokafolio closed 9 months ago

mokafolio commented 9 months ago

Is your feature request related to a problem? Please describe. I am looking for a way to serialize without using streams. I.e. get the resulting toml/json/yaml string directly without having to use sstr << toml_table; Not sure if it's already there and I missed it but I had a hard time finding it in the reference/samples.

Describe the solution you'd like I just want direct getters to retrieve a toml/json/yaml string such as table.toml_string() and maybe table.emit_toml_string(str); to append to an existing string. Cherry on top would be if it could be a custom std::basic_string type for the latter to allow for custom allocators.

marzer commented 9 months ago

There's no way to do this currently, no. A few people have asked me for this here and there over the last few years, so I might add it in the next time I do work on the library. Not sure when that would be, though - it's largely in maintenance mode at the moment.

In the mean-time it's trivial to add as a helper function to your own code:

std::string to_string(const toml::node& node)
{
    std::stringstream ss;
    ss << node;
    return ss.str():
}

table.emit_toml_string(str); to append to an existing string

What's the value-add of providing a separate append functionality? std::string has append operations built-in. Those would suffice methinks.

Cherry on top would be if it could be a custom std::basic_string type for the latter to allow for custom allocators.

While an appealing idea, it's not as compelling as you might think for a library like this one. People use custom allocators for two reasons:

  1. Performance
  2. Special allocation requirements (e.g. no heap)

RE [1]: perf concerns for config file libraries aren't really worth worrying about IMO, beyond anything egregious (the library is already quite fast but I wouldn't be using it in a hot loop in a game engine, for example). Config manipulation is usually done at some I/O boundary, and I/O is usually thousands of times slower than anything you'll find happening in a TOML -> stringstream conversion.

RE [2]: those people are already SOL completely with this library, since I haven't added support for custom allocators anywhere. It'd be a huge job to implement that at this stage 😅

mokafolio commented 9 months ago

Those are all fair points. That said, streams are banned from many codebases and there are other reasons than performance or allocation requirements for custom allocators such as tracking memory usage, memory debugging etc..

Having a separate "append" function allows you to use a string with a custom allocator without you having to make your whole library allocator aware if that makes sense.

Anyways, appreciate the response and I get it's a PITA to add proper allocator support after the fact but I wish more library devs would consider it from the start (no offense, library looks super solid all in all! :) ).

marzer commented 9 months ago

streams are banned from many codebases

Worth nothing that any implementation of this I add to the library will just be a wrapper around the current infrastructure, like the example above, so if you are in a "all iostreams are banned unconditionally" situation then I can't really help.

are other reasons than performance or allocation requirements for custom allocators such as tracking memory usage, memory debugging etc.

Yes, of course there are other use-cases. The two I listed are the main ones. Everything else is very niche in 'business logic' code (doubly so given how good valgrind is), and that's largely where config parsers live.

Having a separate "append" function allows you to use a string with a custom allocator without you having to make your whole library allocator aware if that makes sense.

Ah, ok, so the function would be a template that would then be aware of whatever the allocator was in the input string argument. That makes sense.

I wish more library devs would consider it from the start

I did consider it from the start, and rejected it immediately. The main problem with supporting custom allocators in C++ is their template interface; for a library like this to support custom allocators, it would need to be entirely templated. Not normally a problem for a header-only library, but this one also allows you to compile it "normally" if you so choose, to keep compile times down, et cetera. That becomes impossible if everything is templated, because there's nothing to precompile :sweat_smile:

There is the stuff in std::pmr which can help here, but IME everyone wanting custom allocators already has their own regular allocator and haven't adapted to using std::pmr::polymorphic_allocator, so even if I supported that, I'd still get requests from people wanting full custom allocator support anyway.

It's one of those situations where the benefit to 5% of use-cases isn't at all worth the pain it causes in maintenance to me to help the leftover 95%, heh.

Sorry, I'm not trying to rain on your parade, necessarily, it's just that the library has a large body of users, and any addition I make needs to focus on the general case more than specific ones.

mokafolio commented 9 months ago

Makes sense! By allocator support I did not necessarily mean doing it the templated std::allocator way as I get that is a pretty big tradeoff if the class is not templated to begin with. I agree that a runtime interface such as pmr would satisfy most of the needs of people that wan't to do custom memory things and imo that would be enough and I personally think it's okay to provide only that and not the whole std::allocator BS.

That said, I also understand that even adding a pmr like allocator will most likely make some people mad because the overall performance might loose a couple of nanoseconds 😅.

Since we diverged a little bit from the original issue, feel free to close it if you think adding extra string getters is out of the question.

marzer commented 9 months ago

Since we diverged a little bit from the original issue, feel free to close it if you think adding extra string getters is out of the question.

Well, I guess that depends on you; would the std::stringstream-based wrapper function above suffice? That's about the only way I can swing it, without re-writing much of the underlying formatter code.

mokafolio commented 9 months ago

I think I am good for now, I can do that step myself and mainly just wanted to make sure that I am not missing anything. I will reopen if I change my mind. Thanks for the chats!