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

Schematron: text() node fails #396

Closed
shudson310 opened this issue Oct 9, 2018 · 17 comments · Fixed by #411
Closed

Schematron: text() node fails #396

shudson310 opened this issue Oct 9, 2018 · 17 comments · Fixed by #411

Comments

@shudson310
Copy link

I get a failure when a rule context is a text node: text(). For example:

<sch:pattern>
    <sch:rule context="text()">
        <sch:report id="above-mentioned"
            test="matches(., 'above-mentioned')"
            role="info" >Forbidden word: above-mentioned</sch:report>
    </sch:rule>
</sch:pattern>

<x:scenario label="above-mentioned" focus="test this framework">
    <x:context><conbody><p>above-mentioned</p></conbody></x:context>
    <x:expect-report id="above-mentioned" role="info" location="/conbody[1]/p[1]"/>
</x:scenario>

If I change the rule to match the p element than the XSpec begins to work. I cannot change my .sch for many of my rules, so the XSpec framework should accommodate text() node matches.

@AirQuick
Copy link
Member

AirQuick commented Oct 9, 2018

When I insert this pair of x:param

<x:description schematron="test.sch" xmlns:x="http://www.jenitennison.com/xslt/xspec">
	<x:param name="only-child-elements">false</x:param>
	<x:param name="visit-text">true</x:param>
	<x:scenario focus="test this framework" label="above-mentioned">
...

and run XSpec command line (bin\xspec.bat -s or bin/xspec.sh -s, not Ant), then the test succeeds.


Schematron/schematron#45
Schematron/schematron#46

@AirQuick AirQuick changed the title text() node fails Schematron: text() node fails Oct 9, 2018
@cirulls
Copy link
Member

cirulls commented Oct 10, 2018

@shudson310 : does @AirQuick 's reply answer your question? Can we close the issue?

@shudson310
Copy link
Author

The suggested params do fix the issue, but you may want to emphasize this in the documentation and tutorials. Thanks! You can close this issue.

@AirQuick
Copy link
Member

@vincentml
I think XSpec should set allow-foreign=true, only-child-elements=false and visit-text=true for convenience, unless specified otherwise in /x:desciption/x:param. What do you think?

I have such changes both for command line and Ant.

@AirQuick AirQuick reopened this Oct 10, 2018
@shudson310
Copy link
Author

Sorry to report, but I have tried all of these:
<x:param name="only-child-elements">false</x:param>
<x:param name="visit-text">true</x:param>
<x:param name="allow-foreign">true</x:param>

Still getting errors reported in oXygen plugin. Not sure if it is an XSpec issue or a plugin issue.

@AirQuick
Copy link
Member

Both oXygen transformation scenario and plugin are Ant based. Parameters aren't implemented there yet.
I'll soon submit a pull request for it.

In the meantime, you have to modify [oXygen installation]/frameworks/xspec/build.xml (in case of transformation scenario) or [Home]/.com.oxygenxml/extensions/.../oxygen-results-view/build_report.xml (in case of plugin) to hardcode only-child-elements and visit-text just like allow-foreign is hardcoded:

xspec/build.xml

Line 214 in 7361f19

<param name="allow-foreign" expression="true"/>

@shudson310
Copy link
Author

Thanks for the tip!

On OS X, this was in [Home]/Library/Preferences/com.oxygenxml/extensions/.../oxygen-results-view/build_report.xml

After making this change and adding the parameters, the <x:expect-report> still fails.

Are there any other files that still need to be modified? The expected diff still shows "()" instead of no content.

@AirQuick
Copy link
Member

After modification, your build_report.xml should look like this:

...
    <echo message="   STEP 3: Convert Schematron to XSL"/>
...
      <factory name="net.sf.saxon.TransformerFactoryImpl"/>
      <param name="allow-foreign" expression="true"/>
      <param name="only-child-elements" expression="false"/>
      <param name="visit-text" expression="true"/>
      <param name="phase" expression="${xspec.phase}" unless:blank="${xspec.phase}"/>
      <xmlcatalog if:set="catalog">
...

No other change was necessary on my end.

@vincentml
Copy link
Collaborator

I think XSpec should set allow-foreign=true, only-child-elements=false and visit-text=true for convenience, unless specified otherwise in /x:desciption/x:param. What do you think?

It is better that XSpec itself should not override default parameter settings for Schematron, but users of XSpec can specify parameter settings for Schematron using <x:param>. For example, if someone has not specified Schematron parameters in their environment the behavior of XSpec will be the same. Likewise, if someone has specified parameters for Schematron in their environment they can specify the same parameter values in XSpec. This would, I think, match user expectation.

However, it becomes more dicey when looking at the default values in different implementations of Schematron. The one in the GitHub Schematron project, which bundled with XSpec, is older. The one in oXygen is based on the one from GitHub and contains a number of enhancements. They have each different defaults.

param GitHub oXygen
allow-foreign FALSE FALSE
only-child-elements false, conditional true true, conditional false
visit-text FALSE TRUE

It would be interesting to find out if the enhancements that oXygen has made to Schematron could be contributed to the GitHub Schematron project. This might be a question for @AlexJitianu

What I think we can do for XSpec is to add more information in the wiki documentation about Schematron parameters.

@AirQuick
Copy link
Member

Thanks, @vincentml
Then I won't set any default in my upcoming pull request.

allow-foreign on Oxygen seems to be TRUE by default in its Preferences - XML - XML Parser - Schematron.

@vincentml
Copy link
Collaborator

allow-foreign on Oxygen seems to be TRUE by default in its Preferences - XML - XML Parser - Schematron.

That is a good observation. In the oXygen Preferences panel the defaults for allow-foreign is true. I was looking in Oxygen XML Editor 20/frameworks/schematron/impl/iso_schematron_skeleton.xsl which has false as the default for allow-foreign.

@AlexJitianu
Copy link

AlexJitianu commented Oct 11, 2018

It would be interesting to find out if the enhancements that oXygen has made to Schematron could be contributed to the GitHub Schematron project. This might be a question for @AlexJitianu

Well, we usually create issues/pull request on the GitHub Schematron project with all the enhancements we consider worth making. We did the same with visit-text/only-child-elements parameters but, because we considered that the defaults should change, we didn't wait for the issues to reach a resolution and we made these changes in our distribution of Schematron.

@AirQuick
Copy link
Member

AirQuick commented Oct 13, 2018

@vincentml

If sch:rule has context="text()", x:expect-report/@location doesn't seem to work even when only-child-elements=false and visit-text=true, because of incorrect svrl:successful-report/@location.

To get it work, you seem to have to inject the following template into one of the stylesheets:

   <xsl:template match="text()" mode="schematron-select-full-path">
      <xsl:apply-templates mode="#current" select="parent::element()"/>
      <xsl:text>/text()[</xsl:text>
      <xsl:value-of select="count(preceding-sibling::text()) + 1"/>
      <xsl:text>]</xsl:text>
   </xsl:template>

Can you confirm it? What shall we do?

@AirQuick
Copy link
Member

I submitted #411. With it, /x:description/x:param works on XSpec for Schematron when using Ant.
If you like to test it now on the oXygen transformation scenario, clone the PR branch and follow the steps written in build.xml.

Once the PR gets merged and ported to the oXygen XSpec plugin, x:param will probably work on it, too.

Unfortunately the @location problem needs to be fixed in the upstream, as I wrote in the previous comment.

@AirQuick
Copy link
Member

Just FYI. Using schxslt, x:expect-report/@location seems to work with text nodes. You don't need to set x:param.

@vincentml
Copy link
Collaborator

I had not seen schxslt before. Very interesting! I will have to try using this Schematron implementation.

@AirQuick
Copy link
Member

I added a Wiki page, Testing Schematron with Text Nodes, based on this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants