-
Notifications
You must be signed in to change notification settings - Fork 52
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
Allow the visitor to cease callbacks #88
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Thanks @Vbbab, looks good! I made some small code polishes. |
aeschli
approved these changes
Jun 24, 2024
jrieken
approved these changes
Jun 24, 2024
Alright, thank you so much! |
This was referenced Sep 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As per #30, this PR implements a new feature whereby the callbacks for
onObjectBegin
andonArrayBegin
for theJSONVisitor
may return aboolean
(rather than an arbitrary ignored value as was previously the case) which indicates whether the visitor should continue receiving callbacks for the subtree.As an example, consider the following JSON:
If, upon receiving the
onArrayBegin
callback from the array ofb
, the user returnsfalse
from the callback, then all other callbacks for everything within the array will be suppressed, and the next callback the user receives will only be the correspondingonArrayEnd
.Internally, this works by keeping a counter
suppressedCallbacks
withinvisit()
, which keeps track of the "depth" of the subtree where callbacks are being suppressed. When a newonXXXBegin()
callback is called and this depth is nonzero, it is incremented. Similarly, when anonXXXEnd()
callback is called, the depth is decremented, and callbacks are only called when the counter is0
. This also meant creating new callback adapters (?) such astoBeginVisit
andtoEndVisit
that handle this counter logic on top of wrapping the actual callback. These were modeled after the originaltoNoArgVisitWithPath
/toNoArgVisit
functions.FWIW, I've changed the return type of the two begin callbacks to
boolean | void
rather than justboolean
, so as to preserve some backward compatibility for users who weren't just returning garbage from the callbacks. However, this also meant some refactoring to other parts of the codebase that didn't quite comply with a strictvoid
return.I've also added additional parameters to the
assertVisit
function to test stopping callbacks at a certain position within the JSON (as long as anonArrayBegin
oronObjectBegin
callback can be triggered at the specified position). In order to adapt the fact that the begin callbacks now returnboolean
s, I added abeginHalder
which handles the logic of returningfalse
when appropriate.I'd appreciate it if you would be kind enough to review my code. Let me know what you think!
Thank you!