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

Pretty print support #138

Open
Naios opened this issue Oct 11, 2020 · 3 comments
Open

Pretty print support #138

Naios opened this issue Oct 11, 2020 · 3 comments

Comments

@Naios
Copy link

Naios commented Oct 11, 2020

4f31b90

It would be great if the serializer could also support pretty printing for making the generated toml more readable (the nlohmann::json library supports it as well and it's a feature I don't want to miss for readable serialization).

Currently the serializer generates compact toml as shown below:

#some a
a = 10
#some b
#with more lines
b = 11
ar = [1.0,2.0,3.0]
sv = [
{a=300,b=400},
{a=500,b=600},
{a=700,b=800},
]
#some struct
[s]
a = 100
b = 200
[other]
e = 500
r = 600

The readability could easily improved by:

  • Adding an indentation to arrays spanning more than one line.
  • Inserting a newline before a table (and its comments)
  • (Inserting a whitespace after # when the comment is not empty)
# some a
a = 10
# some b
# with more lines
b = 11
ar = [1.0,2.0,3.0]
sv = [
  {a=300,b=400},
  {a=500,b=600},
  {a=700,b=800},
]

# some struct
[s]
a = 100
b = 200

[other]
e = 500
r = 600

Probably this requires minimal implementation effort. Maybe the formatting rules could be configured nicely through a formatting config that is passed to toml::format:

struct Formatting {
  std::uint32_t indentation{0};
  bool newline_before_table{false};
  bool comment_leading_whitespace{false};

  // ... and maybe more options in the future
};

// ...

toml::format(data, 80, 17, false, false, Formatting(2, true, true));
@ToruNiina
Copy link
Owner

That's nice. Actually, I have been planning to implement almost the same thing.

Since, in my plan, all the formatting parameters including line width and precision are integrated in the struct, it introduces breaking changes. So I suspended it until v4.

However, as you pointed out, the feature itself can be implemented keeping the current interface. I know the feature is needed, so I will implement it. But it could take some time.

@Naios
Copy link
Author

Naios commented Oct 13, 2020

Great 👍🏻 , thank you in advance.

If you implement it with the struct, as suggested, there will be no breaking change since the default constructed struct produces the same output as before and can even be defaulted in the format function signature such that a breaking change can be avoided.

One small performance hint about the serializer: I saw that you are passing a lot of std::strings as return types from the visitor and append those to each other. Since the visitor is doing a pre-order traversal it is probably possible to write your output to a growable buffer (std::vector<char> or sth. else) directly instead. This would be much better in terms of performance instead of doing lots of small allocations for all the returned strings.

@ToruNiina
Copy link
Owner

If you implement it with the struct, as suggested, there will be no breaking change since the default constructed struct produces the same output as before and can even be defaulted in the format function signature such that a breaking change can be avoided.

Right. That is exactly why I have planned almost the same thing, actually.

Since the visitor is doing a pre-order traversal it is probably possible to write your output to a growable buffer (std::vector or sth. else)

Yeah, that's true.
The reason why the current implementation is not in that way is that, at the very beginning, I thought that toml::serializer itself would be used with toml::visit. If so, each operator() should return a string. But after a while it turned out that using serializer directly is not very convenient and then toml::format is implemented. And, since I didn't use serializer heavily in my personal projects, I just left it as is.
Performance sometimes matters, so I will change the implementation in a way you suggested. But it might change the interfaces of serializer, that means that is a breaking change, so it could be in v4 and take much time (to have enough spare time).

Well, anyway, I will do those things when I have time.
(Although I said I will do it, I don't intend to refuse pull request about this, just to make it sure.)

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

No branches or pull requests

2 participants