willemdj / erlsom

XML parser for Erlang
GNU Lesser General Public License v3.0
265 stars 103 forks source link

Use a fake group type for disambiguating between alternatives, similar to what is done for simple types in alts. #77

Closed ElectronicRU closed 4 years ago

ElectronicRU commented 4 years ago

Also necessitates small changes in the parser to be able to handle top-level fake types.

COMPATIBILITY BREAKING!

Closes #76 .

ElectronicRU commented 4 years ago

As a side note, gexf tests apparently have the schema include each element twice in the document root -- not sure if this is intentional or not, but helped me catch a bug, so hey, no complaints.

If you're not keen on breaking compatibility of data in this edge case, I think a flag could be added to the model for this.

willemdj commented 4 years ago

Can you provide a minimal example/test case?

How exactly is this compatibility breaking? Which scenario's would stop working?

willemdj commented 4 years ago

Just noticed that you provided an example in another comment. I'll add it to my set of test cases, and run the entire suite to check if your change breaks anything. Probably not before the weekend, I'll let you know.

ElectronicRU commented 4 years ago

Good, thanks! @willemdj it's compatibility breaking in a sense that scan results change, as well as compilation results for a schema. A pre-compiled schema would continue to work exactly as before, though.

willemdj commented 4 years ago

I ran my tests, and they all work, so that is good news.

However, it is still not clear to me how your change breaks compatibility. Can you give an example?

ElectronicRU commented 4 years ago

I ran my tests, and they all work, so that is good news.

However, it is still not clear to me how your change breaks compatibility. Can you give an example?

Well, the schema types for this example XSD files would be different -- each of the option types would have just one field (and maybe anyAttrs), instead of a copy of fields of underlying type.

Assuming a compiled schema from an older version is used, this should be compatible.

ElectronicRU commented 4 years ago

Hey, don't mean to nag, but what's the status on this currently?

willemdj commented 4 years ago

Don't worry, I'll accept it. I just need to work out whether this should be a new version, things like that - it has been a while since I last had to process a change... I'll look into it this weekend.

Op do 15 okt. 2020 om 09:37 schreef Alex Es notifications@github.com:

Hey, don't mean to nag, but what's the status on this currently?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/willemdj/erlsom/pull/77#issuecomment-708962324, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABGHR6M2TB6MOIHENDNKA3SK2REJANCNFSM4R5HY4EA .