add Key() method to the Handler concept
#134
Conversation
For more details see: Tencent#132 This commit tries to minimize the required code changes and forwards the `Handler::Key()` calls to `Handler::String()` wherever possible in order to not break existing code; or at least not code deriving from `BaseReaderHandler` when implementing a custom `Handler`.
|
This is the absolute minimal set of changes I can come up with to get all the unit tests compile and pass as well as all the sample snippets... |
| @@ -17,6 +17,7 @@ struct MyHandler { | |||
| return true; | |||
| } | |||
| bool StartObject() { cout << "StartObject()" << endl; return true; } | |||
| bool Key(const char* str, SizeType length, bool copy) { return String(str, length, copy); } | |||
There was a problem hiding this comment.
MyHandler should inherit from BaseReaderhandler (which would provide this forwarding already):
struct MyHandler : BaseReaderHandler<UTF8<>, MyHandler> {Secondly, if you add the explicit handler function, you should probably print "Key(" ...
There was a problem hiding this comment.
as I said: this is the absolute minimum to get started.
now the door is open for improvements...
There was a problem hiding this comment.
@pah do you suggest to derive all the custom Handlers that needed the forwarding Key() -> String() call from BaseReaderHandler?
There was a problem hiding this comment.
Yes. I would suggest this as the preferred solution to the API change in the documentation. And we should eat our own dog food.
| } else { | ||
| if (!handler.String(stack_.template Pop<typename TargetEncoding::Ch>(stackStream.length_), stackStream.length_ - 1, true)) | ||
| RAPIDJSON_PARSE_ERROR(kParseErrorTermination, s.Tell()); | ||
| } |
There was a problem hiding this comment.
one is calling handler.Key() and the other handler.String() ...
There was a problem hiding this comment.
I'm calling Key() and String() in my comment, too.
…leanups and additions
It was a copy-n-paste error for the last argument of `Key()` and `String()`...
|
LGTM now. 👍 |
add `Key()` method to the `Handler` concept
Inherit from BaseReaderHandler<> to properly handle calls to the new `Key` function after updating RapidJSON to a current version (see Tencent/rapidjson#132, Tencent/rapidjson#134).
Inherit from BaseReaderHandler<> to properly handle calls to the new `Key` function after updating RapidJSON to a current version (see Tencent/rapidjson#132, Tencent/rapidjson#134).
For more details see: #132
This commit tries to minimize the required code changes and forwards the
Handler::Key()calls toHandler::String()wherever possible in order to not break existing code; or at least not code deriving fromBaseReaderHandlerwhen implementing a customHandler.