julianandrews / sgf-parse

SGF parsing library for Rust.
MIT License
14 stars 3 forks source link

Fix invalid property handling #10

Closed julianandrews closed 3 years ago

julianandrews commented 3 years ago

Currently sgf-parse will throw an error if a property is present, but invalid. The spec says such properties should be dropped or corrected.

I think the exact handling of an invalid property should be left to the application. Rather than throw an error on an invalid property, I think the property should be returned as Invalid which will be otherwise identical to Unknown. Invalid will be used for properties that are expected to have a specific format for the game being parsed, but parse incorrectly.

julianandrews commented 3 years ago

This issue is a split from issue #8

kurnevsky commented 3 years ago

Sounds reasonable. It's the way such properties handled in sgf-parser crate: https://docs.rs/sgf-parser/2.6.0/sgf_parser/enum.SgfToken.html - it has unknown and invalid tokens. Here how I used it, just in case you are interested: https://github.com/pointsgame/oppai-rs/blob/master/iced/src/sgf.rs

julianandrews commented 3 years ago

This should be fixed in 2.0.3. It would be great to know if this works well enough for your use case (I think it probably should, but confirmation would be nice!).

I do like the idea of fixing issue #9 at some point, but that's going to require a little more thought to decide what the new API should look like. In particular, I would like to avoid adding too much boiler plate for Go users since I suspect that's the most common user, and it's not obvious to me immediately how to do that while allowing for multiple different Move/Point/Stone types.