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

Update Json Graph Traversal Algorithm #431

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

acpaquette
Copy link

@acpaquette acpaquette commented Nov 14, 2024

This PR updates the JSON traversal algorithm to build the underlying graph structure that jsoncrack uses. There are a couple of changes that also address a bug in graph construction. Below is a screenshot better detailing the changes:

Screenshot 2024-11-14 at 1 02 35 AM
  • I added a root node to the tree. This makes the structure of the graph a little clearer and allows the "Member" property to remain at the same level as its "brother" properties. This root node also allows the graph structure to parse consistently from the root elements to the leaves.
  • Arrays now construct empty child nodes for arrays of objects or nested arrays. We see that in the "Members" array of objects and in the nested array of powers for the first member. This ensures that all elements within a list of objects remain at the same level and that nested arrays appropriately display in the graph.

Fixes #385

Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tweaked human facing punctuation, spelling, etc.

@AykutSarac AykutSarac added the open to review Please share your insights about this issue/PR label Nov 17, 2024
@paimagham
Copy link

Can I work on this.

@AykutSarac
Copy link
Owner

Good job! It doesn't seem to work with arrays right now.
Also let's take feedbacks from the community :)

@acpaquette
Copy link
Author

@AykutSarac Can you provide the failing array case that you found? Or is it the way arrays are being unpacked?

@AykutSarac
Copy link
Owner

@AykutSarac Can you provide the failing array case that you found? Or is it the way arrays are being unpacked?

It happened when I wrapped the default JSON within an array [].

@acpaquette
Copy link
Author

acpaquette commented Nov 26, 2024

@AykutSarac I think I fixed the array issue you pointed out! Thanks for assistance with testing the change. Here is the format of the json when we wrap the whole example in an array:
Screenshot 2024-11-26 at 12 24 15 PM

Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trivial, human-facing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open to review Please share your insights about this issue/PR
Projects
None yet
4 participants