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

Add indexed-repeat tests #776

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lognaturel
Copy link
Member

@lognaturel lognaturel commented Jul 8, 2024

TODO:

What has been done to verify that this works as intended?

These are test-only additions intended to capture the intended indexed-repeat behavior. It also updates the function's comment to better reflect the language that we use these days to talk about repeats.

I wasn't there when the function was added by my understanding is that it was introduced so end users wouldn't have to write XPath predicates in the common case where they want to use a numeric index to access a specific repeat instance's value. XPath predicates are harder to reason about because the predicate expression is evaluated in the context of the path that's being filtered. It's particularly baffling when using parallel repeats because you end up having to write something like position(.) = position(current()/..).

The indexed-repeat function makes it so the index expression is evaluated using the bound node's context which is intuitive to users. The first parameter is the full reference to a field that needs to get indexed, and then there are some number of pairs of repeat references and indexes.

A simple algorithm to think about the intended behavior (not exactly what JR does):

  • contextualize all the parameters based on the single node binding
  • evaluate all the index parameters (param 3, 5, 7, etc), these should now be literal integers
  • for each repeat reference parameter (param 2, 4, 6, etc), add a positional predicate at that reference level in the base reference (param 1)

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Do we need any specific form for testing your changes? If so, please attach one.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

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