anansi-project / comicinfo

ComicInfo.xml's new home
https://anansi-project.github.io/docs/category/comicinfo
MIT License
136 stars 8 forks source link

Remove tag ordering restriction #29

Open h3xx opened 2 years ago

h3xx commented 2 years ago

Let the document specify the tags immediately under <ComicInfo> in any order, instead of in sequence.

The XSD should allow BOTH forms of:

Should validate:

<ComicInfo>
    <Series>Series 1</Series>
    <Number>Number 1</Number>
    <Title>Title 1</Title>
</ComicInfo>

Should not validate:

<ComicInfo>
    <Title>Title 1</Title>
    <Title>Title 1</Title>
</ComicInfo>

Closes #10

ThePromidius commented 2 years ago

Shouldn't maxOccurs="unbounded" be "1"?

h3xx commented 2 years ago

Shouldn't maxOccurs="unbounded" be "1"?

No, maxOccurs="1" limits it to one element, i.e. EITHER <Title>...</Title> or <Series>...</Series>

I tried validating

This document ``` Title 1 Series 1 Number 1 1 1 AlternateSeries 1 AlternateNumber 1 1 Summary 1 Notes 1 2000 1 1 Writer 1 Penciller 1 LanguageISO 1 Everyone Format 1 Yes Genre 1 Imprint 1 Locations 1 Manga 1 PageCount 1 Publisher 1 ScanInformation 1 SeriesGroup 1 StoryArc 1 Teams 1 Web 1 Characters 1 Editor 1, Editor 2 Editor 3 Translator 3 Colorist 1 Inker 1 Letterer 1 Letterer 2, Letterer 3 CoverArtist 1 ```

with maxOccurs="1" and it complains about <Series> being there even once:

comicinfo.xml:7: element Series: Schemas validity error : Element 'Series': This element is not expected.

But perhaps you're right about <choice> not being optimal. Let me test if repeated elements work as-expected. Maybe I need to get rid of the <choice>. Let me test.

ThePromidius commented 2 years ago

I just tested with the your sample xml and it had some errors. I think i confused the tag. I tested in a random online validator and it did indeed present the error you mention. This was also discussed in #10 I believe choice is not optimal either. The <choice> indicator specifies that either one child element or another can occur: Per w3s this implies that only one would be chosen. At least that's how i understand it.

h3xx commented 2 years ago

I just tested with the your sample xml and it had some errors.

Yeah, the document I put in the comment still has some validity issues, but using the XSD in this project, I'm working through them. See my updated PR description for some samples.

So, through some more testing, I find I had some issues with the schema I first pushed.

I managed to fix both the ordering restriction and the dupe rejection issues in 4c076b16ff6801f15a56ee7d3825ec28444bffad, using <all>

h3xx commented 2 years ago

TODO: I want to: - Help get CI working in order to test this change Edit: Done! (See #30) - Add a test Edit: Done! (See #30) - Change the rest of the XSD's to reflect this change Edit: Done!

ThePromidius commented 2 years ago

You can push to main in your fork to run CI that's what i usually do. It does validate btw.

h3xx commented 2 years ago

It does validate btw.

The XSD validates, that's great, but I meant something more in-depth, like "here is a ComicInfo.xml file that should validate using the drafts/v2.1/ComicInfo.xsd schema" and "here is one that should not validate".

ThePromidius commented 2 years ago

Oh that's right i just validate locally so never payed attention to that.

h3xx commented 2 years ago

Okay, I brought the change into the main schemas, and wrote a script to run a gauntlet of tests (see #30).

gotson commented 2 years ago

Thanks @h3xx , but as mentioned in #10 no parser really cares about the sequence until now. We also did not change it yet, because it could have adverse impacts, like here.