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

Implement @hcayless solution #1 for issue @2246 — do not test @spanTo across files #2662

Merged
merged 3 commits into from
Jan 20, 2025

Conversation

sydb
Copy link
Member

@sydb sydb commented Jan 20, 2025

Since no one has discussed this further in over 2 years, I just went ahead and changed the Schematron (as proposed on the issue) to only test that the target element follows the current element iff the @spanTo starts with ‘#’. I used (what I think) is an easier-to-understand XPath, though.

This is not an ideal solution, of course, because (IMHO) we should allow a user to put whitespace in the attr value and still get the test (i.e., we should use normalize-space() in the code). Also, doesn’t .#duck also point to the element with ID "duck"? For that matter, doesn’t ../me.xml#duck point to it if the current file happens to be called me.xml?

update tests to match.
@ebeshero ebeshero changed the title Impliment @hcayless solution #1 for issue @2246 — do not test @spanTo across files Implement @hcayless solution #1 for issue @2246 — do not test @spanTo across files Jan 20, 2025
Copy link
Contributor

@martindholmes martindholmes left a comment

Choose a reason for hiding this comment

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

Good to go as far as I can see.

Copy link
Member

@ebeshero ebeshero left a comment

Choose a reason for hiding this comment

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

I just pulled in this branch, ran tests and built exemplars so I could see the built RelaxNG. I tested that new version against a local TEI file with a @spanTo that points into external file rather than to a following anchor, and all works as expected. The new version successfully loosens the constraint so we can do this without a validation error. And the new version of the Schematron also works to validate a pointer to a following anchor inside the file. So all's good!

@ebeshero
Copy link
Member

With two positive reviews and a refridge period beginning, I'm just going ahead and merging this now to dev.

@ebeshero ebeshero merged commit 2a2ba2b into dev Jan 20, 2025
3 checks passed
Copy link
Member

@hcayless hcayless left a comment

Choose a reason for hiding this comment

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

@sydb It's not perfect, but I'll point out that the previous rule also assumed a plain fragment identifier. Approved.

@ebeshero ebeshero added this to the Guidelines 4.9.0 milestone Jan 20, 2025
@ebeshero ebeshero deleted the sydb_2246_check_spanning_only_in_current_document branch January 20, 2025 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants