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

GitHub Issue #183 - Improve quoted ns metadata handling #186

Merged
merged 2 commits into from
Feb 27, 2025

Conversation

rap1ds
Copy link
Contributor

@rap1ds rap1ds commented Feb 27, 2025

This PR is built on top of PR #185

This PR improves quoted namespace metadata value handling.

Pull Request #182 added quoted ns metadata value handling for nodes with children (maps, lists etc.) but didn't address all the possible types a wrap node body might have.

This commit checks if the skip target candidate node has children, and if yes it picks the last child and repeats the check until it finds the correct skip target.

Tagged literal values in namespace metadata are missing the hash '#'.

formatNodes function handles the edge case for adding '#' text tot .tag
nodes. However, ns metadata doesn't use formatNodes, instead it collects
the values as text. Thus, we need to add the same edge case handling to
the text collecting.
This commit improves quoted namespace metadata value handling.

Pull Reuqest oakmac#182 added quoted ns metadata value handling for nodes with
children (maps, lists etc.) but didn't address all the possible types a
wrap node body might have.

This commit checks if the skip target candidate node has children, and
if yes it picks the last child and repeats the check until it finds the
correct skip target.
@rap1ds rap1ds changed the title Improve quoted ns metadata handling GitHub Issue #183 - Improve quoted ns metadata handling Feb 27, 2025
@oakmac oakmac merged commit 34967b9 into oakmac:master Feb 27, 2025
1 check passed
@oakmac
Copy link
Owner

oakmac commented Feb 27, 2025

These bug finds + fixes are great! Thank you for the help 🙏

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.

2 participants