pretext icon indicating copy to clipboard operation
pretext copied to clipboard

FITB: multiple fillin inside table cause JSON format error

Open dbrianwalton opened this issue 1 year ago • 4 comments

Commas are missing in the blankNames and feedbackArray fields, presumably because the position() of the fillin is not counting properly to indicate ordering.

dbrianwalton avatar Sep 15 '24 03:09 dbrianwalton

The position() function is a bit dangerous, it counts everything. Can you point me to the location? Perhaps I can make suggestions.

rbeezer avatar Sep 15 '24 17:09 rbeezer

Here are the specific areas I will be checking in pretext-runestone-fitb.xsl. I think there are three specific locations that were not working properly based on the example provided in the pretext-support example. Two are associated with a missing comma being used to generate a JSON array, with each entry meant to match a single fillin element. The third is associated with assigning a name associated with the fillin in the event the author did not provide @name.

Fixing blankNames

<xsl:template match="fillin" mode="declare-blanks"> (line 186) This template is meant to run through all fillin elements inside of a statement and generate an array containing names meant to be associated with the blanks. Those names are used in the javascript that the Runestone Component might use when performing evaluation of user responses. The variable blankNum (line 187) uses position() to associate an order to the fillin elements. If the associated number is greater than 1, a comma is inserted for the creation of the JSON array.

<xsl:template match="fillin" mode="blank-name"> (line 172) When a fillin does not have an assigned @name, a default name is created using blank appended by position().

Fixing feedbackArray

<xsl:template match="fillin" mode="dynamic-feedback"> (line 388) This template is meant to run through all fillin elements inside of a statement and generate the appropriate feedback and tests. The variable blankNum (line 394) is one culprit, set using position() relative to the matching xpath match sent to the template. The missing comma is coming from line 436-438, which was supposed to see if there was a previous match in the tree that would have created an initial element in the array being generated.

The template is called at line 160 using XPATH selector statement//fillin

The variable check is meant to match a corresponding evaluate element that is in the evaluation block. If the fillin is named, it matches by name. Otherwise (and maybe this was a bad choice), it matches by sequential order, so that the first statement//fillin matches the first evaluation/evaluate. The xsl:choose within has a test that uses position() on the evaluate elements to match. I think that if the blankNum was set incorrectly and there is not a match by name, this block is also selecting the wrong evaluate entries, but if the blankNum variable can be fixed, I think this should work again.

dbrianwalton avatar Sep 16 '24 13:09 dbrianwalton

@rbeezer This actually looks like the CLI is behind the actual PreTeXt repo. I actually fixed this error in the following commit: 908d884.

If I run pretext/pretext/pretext to build the sample that has an error, everything works correctly. If I use pretext-cli, the reported error appears.

dbrianwalton avatar Sep 16 '24 14:09 dbrianwalton

Good to hear that the tip is in good shape. Courtney can do an install of the nightly easily if she needs to move forward.

Here is one suggestion. First use of position() above.

You want a comma before each #fillin except the first one. Rather than test for position number, you can test of the fillin has any predecessors.

<xsl:if test="preceding-sibling::fillin">
    <xsl:text>, </xsl:text>
</xsl:if>

(Me, I'd put a comma after each #fillin, unless it was last (no followers)).

But I see you need the actual number, blankNum. You can do something like:

<xsl:value-of select="count(preceding-sibling::fillin)"/>

You could even put the node-set (preceding-sibling::fillin) in a variable early on and reference it twice.

Since nothing is busted, I'll stop here, since I think you will get the idea. Let me know if you want more. This would be a good place to experiment - make some changes like this and see if a diff on output changes. And see if adding in some new nodes (like comment nodes in source!) doesn't mess with the values returned by position().

rbeezer avatar Sep 16 '24 23:09 rbeezer