TEIC / Stylesheets

TEI XSL Stylesheets
228 stars 124 forks source link

Fix: creation of empty `attList` in odd2odd.xsl #630

Closed bwbohl closed 8 months ago

bwbohl commented 9 months ago

addresses #319

This is a local fix for elementSpec and probably these changes should be made in a more general way.

sydb commented 8 months ago

OK. I have now reproduced the basic change you made for elements (putting the output <attList> in a conditional) to attribute lists in classes. I have checked that change into your patch-1 branch (hope you don’t mind), and also merged in the latest from our dev branch while I was there. It is passing all tests (from both Test/ and Test2/) that run on my system.

Can either @bwbohl or @lb42 provide a sample ODD that fails due to an <attList> that has had all its attrs removed? (Then we can test it against the new code, here.)

(BTW, still moderately pessimistic about inclusion in next release, for although this is not a complex fix, there are a dozen others also trying to get into next release :-)

lb42 commented 8 months ago

The teiSimplePrint odd exhibits this and two other problems according to the issue i originally raised some time ago (#319 ... 5 years ago)

sydb commented 8 months ago

OK. I tested using Exemplars/tei_simplePrint.odd (Sorry, @lb42, I had misunderstood). I compared the output of the Stylesheets in this branch against the output of the Stylesheets in the issue_2382_eventName branch (the branch I happened to be on at the time), and the only differences are a) the value of the @source of <schemaSpec> and b) 86 missing empty <attList> elements.

This leads me to believe this is entirely successful, and could be merged.

An incidental finding is that tei_simplePrint.odd is itself invalid. There are duplicate occurrences of 4 different @xml:id values ("page1", "rend-it", "LCSH", & "OTASH").

lb42 commented 8 months ago

Those duplicate xmlid values are however all inside egXML elements where arguably they should be permissible.

HelenaSabel commented 8 months ago

I made a test with and ODD that had the empty <attList> issue and it works great. Thank you @bwbohl!

I also ran Test and Test2 in my local machine and everything passed.

I think this is ready to be merged.