shaack / cm-pgn

Parse and create PGNs (Portable Game Notation for chess games)
MIT License
27 stars 21 forks source link

Make header tags case-insensitive #3

Closed ngocdaothanh closed 6 years ago

ngocdaothanh commented 6 years ago

PGNs like this uses tags all in lower case: https://github.com/DHTMLGoodies/dhtmlchess/blob/master/pgn/tactic-checkmates.pgn

This PR makes header tags case-insensitive.

It also allow including non-official tags, so that if people want to get extra metadata from PGNs, they can do so.

shaack commented 6 years ago

We cannot make the tags case insensitive, the PGN specification says "Tag names, like all symbols, are case sensitive. All tag names used for archival storage begin with an upper case letter." see: http://www.saremba.de/chessgml/standards/pgn/pgn-complete.htm#c8.1 If we make tags case insensitive, we are not compliant with the specification. This may be an issue of https://github.com/DHTMLGoodies/dhtmlchess/blob/master/pgn/tactic-checkmates.pgn

ngocdaothanh commented 6 years ago

So case insensitive tags are not allowed

Currently this library only has features for consuming PGNs, so it's better to make it flexible to accept PNGs like: https://github.com/DHTMLGoodies/dhtmlchess/blob/master/pgn/tactic-checkmates.pgn

So that it can consume as many PGNs as possible. That's the main idea behind the change in this PR.

Later, when we want this library to have features for producing PGNs, we can add a feature to "correct" the bad tags when producing PGNs.

shaack commented 6 years ago

The Problem is that the Tags "event" and "Event" are different tags, because they are case sensive. Like the JavaScript Variables "count" and "Count" are different and it is not possible to correct this automatically, it is just possible to show an error. This is a Syntax error in the PNGs at https://github.com/DHTMLGoodies/dhtmlchess/blob/master/pgn/tactic-checkmates.pgn (If the author wants "event" to be the "Event" tag). It is possible to have an "event", "EVENT" and "Event" tag in the header. But only one has the right meaning.

For me its OK to remove the "&& tags[ret[1]]" at line 35. So every tag will be imported. (We could add a flag like "strictHeaderParsing" in the configuration for this, if not set, every tag is included).

But the Constants at top should remain as they are. If someone names a Tag "event" it is a different Tag than "Event". Specification is specification. If we don`t code as specified the specification is obsolete and the code becomes "wishy-washy".

shaack commented 6 years ago

Later, when we want this library to have features for producing PGNs, we can add a feature to "correct" the bad tags when producing PGNs.

With ".toString()" we already have the function to produce PGNs. There are just missing some convenience functions like "addVariation" or so to create the structure of a PGN. For me, it is important to make the API most similar to chess.js, because everyone knows chess.js. Another important thing is to just implement things which are really needed, to keep the code small and easy.

ngocdaothanh commented 6 years ago

OK, after we allowing non-official header tags to be included in the parse result, the library is flexible enough to use with non-standard PGNs.