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

Feature/parent operator #84

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mattmart3
Copy link

This PR is to introduce the parent operator '^' (issue #80). The implementation is quite simple: it uses a HashMap to store the parent nodes of every visited Value in the tree. When the parent token is later evaluated, all the parent nodes of the current result vector are retrieved. An alternative approach could have been to introduce a new struct (e.g. struct Node) composed of the Value element reference and its parent node reference. Whilst this latter approach would likely have been performed better it would have been way more destructive for the current code base.

I hope you can find this useful and consider I am Rust beginner, so feel free to make any suggestion in order to improve the PR.

Such hashmap needs to be propagated across the whole tree visit.
For each node Value, its parent is saved in the hashmap where the key is
the node Value pointer. When the parent operator is later evalutaed,
all the parent nodes of the current result vector are retrieved.
@freestrings freestrings added the enhancement New feature or request label Nov 25, 2021
@freestrings
Copy link
Owner

This issue is about to be added to the additional feature. (Note #75)
I'm cleaning up the related code. wait please. :)

@mattmart3
Copy link
Author

mattmart3 commented Dec 1, 2021

Thank you for the feedback @freestrings. If I understand correctly you are considering to enable this feature based on a setting parameter, right?
Anyway I've just updated this PR with a fix on the parser, some more tests and a minor improvement for the hasmap use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants