Skip to content

can't add mode="replace" to attribute class #370

Open

Description

@emylonas

Trying to make changes for TEIC/TEI#1867. This works for the one element, <constraintSpec> which has@mode="replace" and '@mode="change" seems to work for several other elements. but compile fails when new class is added and attribute definition is given `mode="replace".

Activity

  1. sydb commented on Apr 19, 2019

    @sydb
    Member

    That is, specifying mode="replace" on an <attDef ident="type"> inside a <classSpec> when the class itself is also a member of att.typed (and thus already had @type) causes a build failure:

    /var/tei/P5/p5odds-examples.rng:28131:27: error: duplicate attribute "type"
    

    It shouldn’t.

  2. martindholmes commented on Apr 19, 2019

    @martindholmes
    Contributor

    @sydb and @martindholmes have narrowed down the problem to what's happening in the odds/classatts.xsl file, where some clever processing takes place to handle cases where attDefs in elementSpecs override those inherited from classSpecs of which the elementSpec is a member. This process is not done for classSpecs (yet). Simply editing the match attribute to cause classSpecs to be processed as well as elementSpecs fails because it generates another kind of error, whereby nested attribute classes end up duplicated on elements, so that simple fix doesn't work. However we think that it is possible to fix this by passing classSpecs through the same process, with some modification to the process.

  3. martindholmes commented on Jun 21, 2019

    @martindholmes
    Contributor

    @sydb and @martindholmes went back through the commits and tested the build, and determined that the problem was introduced by the merge at #343; that particular merge contains lots of individual commits, but only one file seems to have been changed:

    https://github.com/TEIC/Stylesheets/pull/343/files

  4. sydb commented on Jun 21, 2019

    @sydb
    Member

    Stylesheets group comes up w/ the following replacement for a template in odds/odd2odd.xsl:

      <xsl:template match="tei:elementSpec[@mode eq 'change']
                          |tei:classSpec[@mode eq 'change']
                          |tei:macroSpec[@mode eq 'change']
                          |tei:dataSpec[@mode eq 'change']"
                    mode="pass0">
        <xsl:variable name="myGI" select="local-name(.)"/>
        <xsl:variable name="myIDENT" select="@ident"/>
        <xsl:variable name="odd2odd-CHANGE-key-replacement"
          select="ancestor::tei:schemaSpec//
          (tei:elementSpec|tei:classSpec|tei:macroSpec|tei:dataSpec)
          [ @mode eq 'change']
          [ $myGI eq local-name(.) ]
          [ @ident eq current()/@ident ]"/>
        <xsl:choose>
          <xsl:when test="count( $odd2odd-CHANGE-key-replacement ) gt 1">
            <xsl:if test=". is $odd2odd-CHANGE-key-replacement[@ident eq $myIDENT][1]">
              <xsl:copy>
                <xsl:copy-of select="@*"/>
                <xsl:for-each select="$odd2odd-CHANGE-key-replacement[ @ident eq $myIDENT ]">
                  <xsl:apply-templates select="node()" mode="pass0"/>
                </xsl:for-each>
              </xsl:copy>
            </xsl:if>
          </xsl:when>
          <xsl:otherwise>
            <xsl:copy>
              <xsl:copy-of select="@*"/>
              <xsl:apply-templates select="*|text()|comment()|processing-instruction()" mode="pass0"/>
            </xsl:copy>
          </xsl:otherwise>
        </xsl:choose>
      </xsl:template>
  5. martindholmes commented on Jun 21, 2019

    @martindholmes
    Contributor

    The above function didn't actually work, but subsequent tests show that the problem with simplePrint is that it consists initially only of specGrpRefs, and those refs are not expanded early enough in the process. So we need a pass before pass0 which will expand all specGrpRefs.

  6. ebeshero commented on Jun 21, 2019

    @ebeshero
    Member

    Quick summary of where we stopped the meeting: We were constructing a variable as a document-node() with guidance from the Kay book, to store the expanded specGrpRefs and pass them to the key() definition as its third argument, which defines its context and must be a document-node. This will, we hope, give us a means to do a pass before pass0!

  7. sydb commented on Jun 22, 2019

    @sydb
    Member

    Do tell, @ebeshero, y’all are talking about using the XSLT 2.0 <xsl:document> instruction, yes? That’s where my head is going right now. I played with it — dividing pass0 into two passes — for perhaps half an hour this morning. But I am no longer convinced this is a stylesheets bug!
    I think it may be a tei_simplePrint problem. Namely, that it (P5/Exemplars/tei_simplePrint.odd) has two <classSpec type="atts" ident="att.global.rendition" mode="change"> elements. Is that supposed to be allowed?
    My instinct is “yes, someday we would love it if multiple mode=change declarations worked, but it doesn’t now and hasn’t for the last decade, so use only one.” Well, at least that’s what I vaguely recall saying in workshops. But it was working just a few months ago, eh? Or was that second <classSpec> added? @martindholmes do you remember? If not, I will try to look that up again later today.
    Anyway, if validated against P5/Exemplars/tei_customization.isosch, this duplicate <classSpec> is not caught, but 4 duplicate <elementSpec>s are. (As well as 4 duplicate @xml:id values, and a bunch of stuff that should not really be a problem.) The error messages are “Current ODD processors will not correctly handle more than one <elementSpec> with the same @Ident” on lines 3430, 4646, 4982, and 5113.

  8. sydb commented on Jun 22, 2019

    @sydb
    Member

    Oh, but that’s silly, isn’t it — this ticket is about mode="replace" in attribute classes other than simplePrint. We were just using simplePrint as a test case yesterday. (Possibly a red-herring test case, though.)

  9. martindholmes commented on Jun 22, 2019

    @martindholmes
    Contributor

    We should definitely fix simplePrint, and add some Schematron to prevent this duplication. It makes no sense in a single ODD file.

  10. martindholmes commented on Jun 28, 2019

    @martindholmes
    Contributor

    @sydb and @martindholmes have added fixes (TEIC/TEI/#6589fca) (along with @lb42 ) to simplePrint to avoid cases where there are multiple *Spec elements with the same @ident. This should fix the build process for the moment.

  11. lb42 commented on Jun 29, 2019

    @lb42
    Member

    This change does cause the make to fail a little further on: something is apparently going wrong in the generation of the -examples.rng file, or the nvdl file which calls it. I can't reproduce the problem using the current "bleeding edge" framework in oXygen, which obstinately goes on doing the right thing, so I am still puzzled.

    BUILD SUCCESSFUL
    Total time: 10 seconds
    ../run-onvdl tei_simplePrint.nvdl tei_simplePrint.odd
    /usr/bin/onvdl
    /home/lou/Public/TEI/P5/Exemplars/tei_simplePrint.odd:606:79: error: attribute "rhyme" not allowed at this point; ignored
    /home/lou/Public/TEI/P5/Exemplars/tei_simplePrint.odd:700:48: error: attribute "part" not allowed at this point; ignored
    
  12. lb42 commented on Jun 29, 2019

    @lb42
    Member

    On closer inspection, these are all genuine errors, or at least discrepancies between what the prose of the TEI Simple doc says is possible, or exemplifies, and what the schema supports.

    I've opened a separate ticket for that at #383 (and closed it again, having fixed the problems)

  13. 7 remaining items

  14. added this to the Release 7.57.0 milestone on Nov 20, 2023
  15. sydb commented on Nov 20, 2023

    @sydb
    Member

    Assigning to @trishaoconnor for the purpose of testing to see what is left to do on this ticket. Quick survey seems to say that the main remaining problem is the HTML output of the test ODD does not correctly reflect that the attribute (@type in this case) has been over-ridden.

  16. trishaoconnor commented on Mar 7, 2024

    @trishaoconnor
    Contributor

    Tested this with @sydb and the results indicated that the schema is correct but the HTML output does not reflect that @type was replaced.

  17. ebeshero commented on Jul 7, 2024

    @ebeshero
    Member

    Revisiting this, I think we have several unresolved tickets that might(?) now be resolved with changes in ODD processing. I think we might want to collect them in a bundle and determine where we are with each reported issue. @emylonas Is this ODD still part of your world? I wonder if we can try it again...

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

Metadata

Metadata

Labels

conversion: oddpriority: highRelatively urgent; Council should revisit routinely.resp: StylesheetsGroupIssue is suitable for the group-learning approach taken in the Stylesheets Coop Working Group.status: inProgressTicket has been assigned and someone is working on it.type: bugA bug report.

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions