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

New tests for TVTs #810

Merged
merged 2 commits into from
Feb 2, 2025
Merged

New tests for TVTs #810

merged 2 commits into from
Feb 2, 2025

Conversation

ndw
Copy link
Contributor

@ndw ndw commented Jan 12, 2025

It seems surprising that there were no tests for inserting XML into text documents with TVTs, but since I was doing it wrong, I assume there weren't.

@xml-project I wonder if you agree with these results?

@ndw ndw requested a review from xml-project January 12, 2025 13:01
@ndw
Copy link
Contributor Author

ndw commented Jan 12, 2025

For the moment, I've special cased inserting attributes, see xproc/3.0-specification#1131

Copy link
Member

@xml-project xml-project left a comment

Choose a reason for hiding this comment

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

I think 002 should raise XD0052, not XD0030.

@ndw
Copy link
Contributor Author

ndw commented Jan 12, 2025

The phrasing of XD0052 suggests that you should be able to use TVTs to add attributes to elements in an XML context. Do you have an example that does that? I can't construct one, but that may be a bug.

@xml-project
Copy link
Member

On thinking about it again: I think "nw-tvt-002" should pass. An attribute (as result of the TVT) is added to the surounding element. So the result, I would expect, is:
<wrapper><copy name="value" /></wrapper>

@ndw
Copy link
Contributor Author

ndw commented Jan 12, 2025

Yes. I think you're right, nw-tvt-002 should pass. I'm just going to leave this PR open until (a) I think I've sorted out my implementation and maybe until (b) we've talked about the spec bug.

@ndw
Copy link
Contributor Author

ndw commented Feb 2, 2025

Except I forgot to change it so nw-tvt-002 passes. Another PR coming in a bit...

@ndw
Copy link
Contributor Author

ndw commented Feb 2, 2025

Okay. I think these are correct. I'm sure @xml-project will tell me if I'm mistaken :-)

@ndw ndw merged commit d859e23 into xproc:master Feb 2, 2025
@ndw ndw deleted the tvt-tests branch February 2, 2025 11:52
@xml-project
Copy link
Member

@ndw wrote:

Okay. I think these are correct. I'm sure @xml-project will tell me if I'm mistaken :-)

I do think to, these are correct. Thank you!

@ndw
Copy link
Contributor Author

ndw commented Feb 2, 2025

👍

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