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

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

Merged
merged 3 commits into from
Nov 9, 2023
Merged

Conversation

bwbohl
Copy link
Contributor

@bwbohl bwbohl commented Oct 4, 2023

addresses #319

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

@sydb
Copy link
Member

sydb commented Oct 26, 2023

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
Copy link
Member

lb42 commented Oct 26, 2023

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

@sydb
Copy link
Member

sydb commented Oct 26, 2023

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

Copy link
Member

@sydb sydb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of the test failures are the SLACK_WEBHOOK thing, so have nothing to do with the actual XSLT. Thus I think if @joeytakeda (or anyone else) can reproduce a successful of an ODD with a known problem, this can just be merged.

@lb42
Copy link
Member

lb42 commented Oct 26, 2023

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

@HelenaSabel HelenaSabel self-requested a review October 26, 2023 19:28
@HelenaSabel
Copy link
Member

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.

@sydb sydb merged commit a855bbe into TEIC:dev Nov 9, 2023
0 of 2 checks passed
@bwbohl bwbohl deleted the patch-1 branch November 9, 2023 21:59
@raffazizzi raffazizzi added this to the Release 7.56.0 milestone Nov 17, 2023
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

Successfully merging this pull request may close these issues.

6 participants