xproc / 3.0-steps

Repository for change requests to the standard step library and for official extension steps
10 stars 7 forks source link

Confusing description for p:replace #635

Closed xatapult closed 2 weeks ago

xatapult commented 2 weeks ago

The p:replace step says:

If the document node is matched, the result is a text document.

But that sounds kinda weird to me. Is that what we mean/meant: replacing the document-node with an XML document gives you a text document???

This makes more sense to me: Replacing the document node with an XML document gives you that XML document, replacing it with a text document a text document. So shouldn't the sentence above read:

If the document node is matched, the result is the replacement document. The content-type is adapted accordingly.

BTW, Morgana implements it the way I think it should be and returns an XML document if I replace / with an XML document.

xml-project commented 2 weeks ago

I think we have a versioning problem here. I think that was fixed with #570.

ndw commented 2 weeks ago

Yes and no. In #570 we find some improvements and this note:

Ammendment: change the wording to "the result is a text document..." with a link to where text documents are defined.

But no evidence that any further changes were made to incorporate that note.

I'll try again!

xatapult commented 2 weeks ago

My suggestion, for what it's worth. At least, this is what I think what was meant:

If the document node is matched and the replacement document is a text document, the result is a text document.

Hmm. This does not address the case when the root element is matched. That might also produce a text document, but what when the source has comments/pi-s outside of the root element? It's a bit tricky...

ndw commented 2 weeks ago

In fact, I think the description of Document properties describes the situation correctly. As @xatapult says, replacing the document element with a text node will also produce a text document provided that there are no comments or PIs floating around outside the document element.

The "If the document node is matched..." paragraph also appears in p:hash, p:string-replace, and p:uuid. I think the bug in PR #570 is that I carelessly copied the same paragraph into p:replace but the situation is more nuanced in p:replace.

I'm torn between removing the paragraph, because the situation is already correctly described in the document properties section, and fixing it for consistency with the other steps. I guess consistency is better and I think Erik's suggestion is a good one.