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

Fix property node's "value" field #5

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nickcoutsos
Copy link
Contributor

I've noticed that the value field in a property node gives = instead of the property's value node. This change just restructures that part of the rule so that the entire value field is optional, and the portion used in the field starts after the = token.

I was a little unsure about this approach since it makes it sound like the presence of the field is optional but this is consistent with the tests.

@nickcoutsos
Copy link
Contributor Author

While looking into fixing an issue with my handling of unit_address nodes I noticed that there's a similar issue with the node's address field returning @. If this fix is alright I'd be happy to make a followup PR to update fields that have optional matches.

@nickcoutsos
Copy link
Contributor Author

It seems the underlying issue here is that the value field was being applied to the = (non-named child) as well, and node.childForFieldName() only returns the first. For my use case I need to use a cursor to access each instance of the field so I can just restrict that to named children anyway, but I think the above fix is otherwise valid.

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

Successfully merging this pull request may close these issues.

1 participant