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

SAX dumper #1512

Closed
Al2Klimov opened this issue Mar 12, 2019 · 12 comments
Closed

SAX dumper #1512

Al2Klimov opened this issue Mar 12, 2019 · 12 comments
Labels
state: please discuss please discuss the issue or vote for your favorite option state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated

Comments

@Al2Klimov
Copy link

Like the SAX parser, but for dumping. A class with methods called by the using app instead of being called.

@nlohmann
Copy link
Owner

The whole logic for dumping is in file https://github.com/nlohmann/json/blob/develop/include/nlohmann/detail/output/serializer.hpp which implements class serializer with constructor

serializer(output_adapter_t<char> s, const char ichar,
           error_handler_t error_handler_ = error_handler_t::strict)

and a public method

void dump(const BasicJsonType& val, const bool pretty_print,
          const bool ensure_ascii,
          const unsigned int indent_step,
          const unsigned int current_indent = 0)

taking that implements the traversal.

Some thoughts:

  • We could add an overload of basic_json::dump that takes a serializer-like object and uses that for serialization. This would mean that if someone wants some user-defined serialization, she must provide the complete implementation for this.
  • Alternatively, we could define a SAX-like interface like dump_string, dump_integer, etc. which separates the logic from the representation. However, I fear that there are so many possible adjustments people want to make to serialization, that it would be impossible to foresee all of them in the interface.

What do you think?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Mar 13, 2019
@Al2Klimov
Copy link
Author

The second one sounds exactly like the issue description.

@nlohmann
Copy link
Owner

@Al2Klimov Right, but there are still a lot of details to decide.

@stale
Copy link

stale bot commented Apr 13, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Apr 13, 2019
@stale stale bot closed this as completed Apr 20, 2019
@Al2Klimov
Copy link
Author

@nlohmann Interesting... is that this feature request's end?

@nlohmann
Copy link
Owner

No, it's just no-one commented on this for 10 days, and I had no time to look into it.

@nlohmann nlohmann reopened this Apr 23, 2019
@stale stale bot removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Apr 23, 2019
@Al2Klimov
Copy link
Author

I'm not stressing you... but I'm afraid the bot does.

@nlohmann
Copy link
Owner

The bot is not stressing me. :)

@Al2Klimov
Copy link
Author

At least it may let some tickets disappear. For me a closed ticket stands for the subject is either done, rejected or a duplicate.

@nlohmann
Copy link
Owner

This is a feature. The project is run by a few people in their free time. A lot of issues are feature requests that may need a lot of time to realize, test, and document. If these issues were left open after months of inactivity, then this would be more of a pressure. And since they are labeled "stale", it is hopefully clear why they have been closed.

@stale
Copy link

stale bot commented May 23, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label May 23, 2019
@stale stale bot closed this as completed May 30, 2019
@dnsmichi
Copy link

dnsmichi commented Jun 3, 2019

Just a heads up, since I wanted to add my thoughts here too: We really appreciate all the work you're pushing into this project. The replacement of the old YAJL library went smooth and all the dependencies in Icinga 2 work better now, including proper UTF8 support. The first release will be 2.11 soon enough, and we'll sure highlight your work in the release blog post.

That being said, this feature request would be nice to have. Since we're all involved in open source projects, time and resources are needed. Given the difficulties with the interface design here, I'm not sure if that works out, especially with later supporting it. I've seen similar problems with Boost Beast and custom parsers lately. Looking at our code base (https://github.com/Icinga/icinga2/blob/master/lib/base/json.cpp), we also have some specialities with custom value types not so common in the C++ world. I'm fine with it, and if we find the time to create a PoC or discuss this in person, we'll see about it.

For now, thanks for taking care :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: please discuss please discuss the issue or vote for your favorite option state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated
Projects
None yet
Development

No branches or pull requests

3 participants