schematron-quickfix / sqf

Schematron Quick Fixes
25 stars 6 forks source link

is there a mechanism to generate SQF'es dynamically? #3

Closed rvdb closed 8 years ago

rvdb commented 9 years ago

I've been experimenting with SQF to assist text editors with entering consistent regularizations for proper names in their XML documents. The idea is that there's an external file linking those regularizations with varying occurrences of the names; when an editor enters a name in an XML document, the value of that name is looked up in the regularizations list, and if a matching entry is found, a SQF should suggest to add the corresponding regularization in the XML file being edited. I'll make abstraction of the exact encoding details; the basic idea is that I want to use SQF to generate encoding suggestions from an open list of values, generated dynamically from some XPath expression.

This works well as long as only one value suggestion is retrieved. Yet, there are cases when multiple values (say "a" and "b") can be returned by the XPath expression. Currently, the most sensible option I've found is having one SQF offer a concatenation of all computed values for later disambiguation. Say, a single fix that offers something like: "add the value 'a|b' to your document". Later on, that concatenated value has to be cleaned up, and only the correct one should remain.

Yet, instead of concatenating those values in a single SQF suggestion, I'd like to be able to split them over separate SQF fixes, or to have one SQF generate multiple encoding suggestions. If values "a" and "b" are found, I'd like to offer two suggestions from which the text editor can choose:

If I understand the SQF syntax correctly, one SQF corresponds to a single suggestion, and each SQF has to be named explicitly in a @sqf:fix attribute Since the list of possible values is computed from a document (and not from, say, a closed value list in a schema), it is impossible to anticipate all possible values and create separate SQF'es in advance. I'm wondering if there is some kind of xsl:for-each concept that can be used to either generate separate suggestions in one SQF, or to generate separate SQF'es dynamically for each of the items in the iteration.

nkutsche commented 9 years ago

Hi, thank you for your feedback! I see your problem, because I have it too. In SQF there are two options to do such multiple fixes for a list of possible parameter values:

Option 1: List of fixes for each item of the list If your list of parameter values (a and b in your example) is short and you know or define a maximum number of parameter items, you can define for each possible position a QuickFix. To hide the fixes, whose position is larger then the current item count, you can use the use-when condition. An example (from the sqf.sch which checks the rules of SQF):

<let name="asserts" value="../(sch:assert | sch:report)"/>
<let name="assertCount" value="count($asserts)"/>
<sch:assert test="..." sqf:fix="add">The fix is not used by any assert/report element.</sch:assert>
<sqf:group id="add">
    <sqf:fix id="add1" use-when="$assertCount ge 1 and $assertCount le 3">
        <sqf:description>
            <sqf:title>Add a reference to the first of the assert/report elements.</sqf:title>
        </sqf:description>
        <sqf:add match="$asserts[1]" target="sqf:fix" node-type="attribute" select="
            string-join((@sqf:fix,
            $id), ' ')"/>
    </sqf:fix>
    <sqf:fix id="add2" use-when="$assertCount ge 2 and $assertCount le 3">
        <sqf:description>
            <sqf:title>Add a reference to the second of the assert/report elements.</sqf:title>
        </sqf:description>
        <sqf:add match="$asserts[2]" target="sqf:fix" node-type="attribute" select="
            string-join((@sqf:fix,
            $id), ' ')"/>
    </sqf:fix>
    <sqf:fix id="add3" use-when="$assertCount ge 3 and $assertCount le 3">
        <sqf:description>
            <sqf:title>Add a reference to the third of the assert/report elements.</sqf:title>
        </sqf:description>
        <sqf:add match="$asserts[3]" target="sqf:fix" node-type="attribute" select="
            string-join((@sqf:fix,
            $id), ' ')"/>
    </sqf:fix>
</sqf:group>

Pro:

Contra:

But: I don't think we should allow to generate unbounded QuickFixes for one Schematron error. This would causes many problems. So see option 2.

Option 2: Define User Entry (not supported in oXygen 17!) You can define a User Entry for that. So the user is able to set the needed parameter by itself, after the choose of the QuickFix:

<sqf:fix id="addGen">
    <sqf:description>
        <sqf:title>Add a reference to one of the existing assert/report elements.</sqf:title>
    </sqf:description>
    <sqf:user-entry name="pos" type="xs:integer" default="1">
        <sqf:description>
            <sqf:title>Please enter the position of the assert/report, which should refer to this fix.</sqf:title>
        </sqf:description>
    </sqf:user-entry>
    <sqf:add match="$asserts[$pos]" target="sqf:fix" node-type="attribute" select="
        string-join((@sqf:fix,
        $id), ' ')"/>
</sqf:fix>

The user defines the position of the assert, which should reference to the QuickFix, by setting the User Entry. Pro:

Contra:

My conclusion: I think the option 1 is for short parameter value lists acceptable. But I don't think it is very helpful, if a Schematron error has more than 10 QuickFixes. For larger parameter value lists, I think we should extend the User Entries, for instance like this:

<sqf:user-entry name="test">
    <!-- ... -->
    <sqf:type>
        <sqf:enum select="distinct-values(//*/name())" base="xs:NCName"/> |  <sqf:pattern pattern="{'\d\.\d\.\d\.'}"/>
    </sqf:type>
</sqf:user-entry>

But I think we should wait for an implementation, which support User Entries, before we extend this structure.

PStellmann commented 9 years ago

Hi,

I have (or at least I will have) the same problem with different use-cases. I wouldn't use option 1 since it doesn't fulfill my personal requirements on "clean code".

While option 2 surely would solve the problem I'd still prefer an option 3: Have one sqf:fix (or maybe sqf:multi-fix) element that generates potentially multiple entries. Assuming there is an xsl function my:getValues() the code could look like this:

<sqf:fix id="setAttributeX">
    <sqf:for-each select="my:getValues()">
        <sqf:description>
            <sqf:title>set attribute x to '<sqf:value-of select="."/>'.</sqf:title>
        </sqf:description>
        <sqf:add match="." target="x" node-type="attribute" select="."/>
    </sqf:for-each>
</sqf:fix>

or

<sqf:multi-fix id="setAttributeX" list="my:getValues()">
    <sqf:description>
        <sqf:title>set attribute x to '<sqf:value-of select="."/>'.</sqf:title>
    </sqf:description>
    <sqf:add match="." target="x" node-type="attribute" select="."/>
</sqf:multi-fix>

I didn't look into the sqf source code yet, but I wouln't expect this to be that difficult to implement. And it could probably be easily supported by Oxygen since you'd only have to replace the xsl files!?

Furthermore for me it feels more consistent (within itself and compared to quickfixes known from other environments like eclipse) to directly choose the desired option and execute it rather than opening a dialog with additional (sub-)options.

rvdb commented 9 years ago

Thanks for your workaround (option 1). For my list of values, this allowed me to even specify a 'fallback' option when lists exceed the maximum length:

  <let name="unique.suggestions" value="distinct-values(//name/@reg)"/>
  <let name="suggestions.count" value="count($unique.suggestions)"/>
  <sqf:group id="reg.replace">
    <sqf:fix id="reg.replace1" use-when="$suggestions.count ge 1 and $suggestions.count le 2">
      <sqf:description>
        <sqf:title>Replace the value of @reg with <value-of select="$unique.suggestions[1]"/>.</sqf:title>
      </sqf:description>
      <sqf:delete match="@reg" use-when="@reg"/>
      <sqf:add match="." node-type="attribute" target="reg" select="$unique.suggestions[1]"/>        
    </sqf:fix>
    <sqf:fix id="reg.replace4" use-when="$suggestions.count ge 2">
      <sqf:description>
        <sqf:title>Replace the vallue of @reg with <value-of select="string-join($unique.suggestions, ' | ')"/>.</sqf:title>
      </sqf:description>
      <sqf:delete match="@reg" use-when="@reg"/>
      <sqf:add match="." node-type="attribute" target="reg" select="string-join($unique.suggestions, '*')"/>        
    </sqf:fix>        
  </sqf:group>

In this (simplified) example, the values are concatenated if there's more than one item. This still provides the editor some assistance: he can enter the entire list and should later on select the desired value manually by removing the others. A very ugly workaround, but of immediate use.

Sqf:user-entry looks promising but it will take some time before an implementation will be available for use-cases like this.

I really like and second PStellmann's suggestion. I had tried to create SQF fixes dynamically via a named XSLT template in my Schematron file, which didn't work (obviously, probably). Some native SQF looping expression would really come to help here. If it could really be implemented at the Schematron-processing XSLT level, that could be a quick win.

PStellmann commented 9 years ago

As a proof of concept I just successfully implemented a prototype and also managed to integrate it into oxygen - So it is definetly working :)

Here is the sample code:

<sch:pattern>
        <sch:rule context="sl">
            <sch:report test="count(sli) mod 2 != 0" sqf:fix="removeAnyItem">
                The list must contain an even number of entries.
            </sch:report>
            <sqf:multi-fix id="removeAnyItem" select="1 to count(sli)">
                <sqf:description>
                    <sqf:title>remove item #<sch:value-of select="$sqf:current"/>: "<sch:value-of select="sli[$sqf:current]"/>"</sqf:title>
                </sqf:description>
                <sqf:delete match="sli[$sqf:current]"/>
            </sqf:multi-fix>
        </sch:rule>
    </sch:pattern>

The fictitious task is to generate a quick fix for each sli item since deleting any of them would solve the "bug".

The current concept of sqf:multi-fix:

I will gladly share my (experimental and only poorly tested) code, if you tell me how.

To integrate it into oxygen you can overwrite the built in xsl files as discussed here: http://www.oxygenxml.com/forum/topic11368.html?sid=6125ee37ef529e945f6fdc89797847c6

PStellmann commented 9 years ago

I had another idea of a syntax for this "multi-fix" that requires much less modification to the current schema and, thus, less modification to the spec and scripts: Instead of a seperate multi-fix element I just added a use-for-each attribute (inspired by the existing use-when attribute).

The sqf code now looks like this:

<sqf:fix id="removeAnyItem" use-for-each="1 to count(sli)">
    <sqf:description>
        <sqf:title>remove item #<sch:value-of select="$sqf:current"/>: "<sch:value-of select="sli[$sqf:current]"/>"</sqf:title>
    </sqf:description>
    <sqf:delete match="sli[$sqf:current]"/>
</sqf:fix>
PStellmann commented 9 years ago

Is there anything else I can do to support this issue? Would be nice to have this added to the next oxygen release as well...

rvdb commented 9 years ago

I'm afraid I can only strongly support your suggestion... seems like an elegant addition to me!

octavianN commented 9 years ago

We need first to decide about the specification. I like the idea to generate multiple fixes.You can solve this with an user-entry but maybe is better to have the fixes displayed directly in the menu and just select the proposal than to have other dialog displayed (using an user-entry). I think both proposals are ok:

PStellmann commented 9 years ago

My personal vote goes to the "@use-for-each" attribute. Understanding the "@use-for-each" attribute as a generalization of the "@use-when" attribute it is actually no huge extension. So I don't see the need to express a big difference to a single-fix by using a different element. And introducing a new "multi-fix" element would also require more work for xslt-modifcation, testing and maintenance (future enhancements to the fix element would probably apply to multi-fix as well).

nkutsche commented 8 years ago

This issue was subject of a concept session during the XMLPrague 2016. Read the full concept paper here.

Following, a conclusion summary and open issues:

Everyone was satisfied by using Patrik's example. Short summary:

Open issues commented by @hschmull: shouldn't this be a function? sqf:current() like current() in XPath?

PStellmann commented 8 years ago

I think a variable is much easier to implement in XSLT. Actually I have no idea how a function could be implemented without using java code or Saxon specific extensions. And there is an analog case in XSLT as well: the err:* variables within an <xsl:catch>.

We might think about making the two attributes @use-when and @use-for-each exclusive so you can use only one of them. It is always easily possible to add a boolean rule to the xpath within @use-for-each. This could make the implementation easier...

nkutsche commented 8 years ago

Yes, I tried to implement a function, but with XSLT (without extensions) it is in my opinion impossible, though I would prefer a function solution. But yeah, we shouldn't define a solution, which is not implementable without XSLT extensions. I was also thinking about removing the variable/function and set the context to the current item. The context of the rule would be easy to save in a variable, defined by the user/schematron developer, but I don't think that is really practical.

We might think about making the two attributes @use-when and @use-for-each exclusive so you can use only one of them.

Hm, I don't see, why it is easier to implement it. For Escali, it will be translated just like this:

<xsl:if test="{@use-when}">
   <xsl:for-each select="{@use-for-each}">
   [...]
   </xsl:for-each>
</xsl:if>

Anyway, if you translate a combination of @use-when and @use-for-each generically, you'll need this:

<sch:let name="bool" value="path/to/check = 'check-value' "/>
<sqf:fix use-for-each="path/to/anywhere/else[$bool]">
[...]

I don't think that is easy for XPath beginners to understand!

PStellmann commented 8 years ago

Anyway, if you translate a combination of @use-when and @use-for-each generically, you'll need this:

<sch:let name="bool" value="path/to/check = 'check-value' "/>
<sqf:fix use-for-each="path/to/anywhere/else[$bool]">
[...]

I don't think that is easy for XPath beginners to understand!

This would work as well (still not as clear as a combination of @use-when and @use-for-each):

<sqf:fix use-for-each="if (path/to/check = 'check-value') then path/to/anywhere/else else ()">

I just thought about the case the the @use-when returns true buzt the @use-for-each returns an empty sequence. In this case there's still no fix generated. But this shouldn't surprise anything.

So let's just forget the idea of making these attributes exclusive...

nkutsche commented 8 years ago

This was already specified in commit 7be4d42.

octavianN commented 6 years ago

Hello,

We just released XML Editor version 20, and we added support to generate multiple similar fixes that are instantiated for each match of the newly added use-for-each attribute. You can read more about this in our user manual: https://www.oxygenxml.com/doc/versions/20.0/ug-editor/topics/sqf-dynamic-fixes.html

Best Regards, Octavian