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

FITB: multiple fillin inside table cause JSON format error #2257

Open
dbrianwalton opened this issue Sep 15, 2024 · 4 comments
Open

FITB: multiple fillin inside table cause JSON format error #2257

dbrianwalton opened this issue Sep 15, 2024 · 4 comments

Comments

@dbrianwalton
Copy link
Contributor

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

@rbeezer
Copy link
Collaborator

rbeezer commented Sep 15, 2024

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

@dbrianwalton
Copy link
Contributor Author

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
Copy link
Contributor Author

@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.

@rbeezer
Copy link
Collaborator

rbeezer commented Sep 16, 2024

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().

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

No branches or pull requests

2 participants