mliebelt / pgn-viewer

Simple PGN viewer with the necessary features to display chess games
GNU General Public License v3.0
161 stars 44 forks source link

peg.js version 0.9.0 #7

Closed steinitz-zz closed 8 years ago

steinitz-zz commented 8 years ago

Hi,

I've made some minor improvements to your peg-rules.peg (see below). My changes are marked with SJS. Feel free to use my changes.

PgnViewerJS uses peg version 0.8.0. Peg has now been updated to 0.9.0 which breaks peg-rules.peg - it throws 'Can't find variable: hm' for valid pgn. I've raised an issue on the peg.js project but wondered if you might have any idea how to fix it. I've spent a few hours on it without success.

Cheers,

Steve

mliebelt commented 8 years ago

As you may have noticed, it is really complicated to get the differences from your version to my version (even if they are really simple). Here the comments (without tooling to validate, that I have seen only the relevant things):

I have not had a chance to analyze the difference between version 0.8.0 and 0.9.0. I have to be more stringent here, though, to be sure, that each change leads to a version change (which is not the case ... currently).

steinitz-zz commented 8 years ago

Hi,

I have been unclear. The commented out parts are your original versions. The version as shown is slightly more robust with some of the pgn I've encountered. Of course you are free to use it or not -- I offer it only as information from 'the coal face'.

While adding, to my site, endgames from Shereshevsky's wonderful 'Endgame Strategy', I fix each pgn parsing issue as I encounter it.

Regarding the version 0.9.0 issue, I received this from @mindgun this morning:

I think, it because from version 0.9.0 the parser is generated in strict mode, and actions of this grammar don't conform to the rules of scrict mode, in particular, use undeclared variables (there is no var keyword before declaration of variables).

It seems a good clue and, perhaps, useful to you too.

Cheers,

Steve

steinitz-zz commented 8 years ago

I've now got a version of your peg-rules.peg which:

Let me know if you'd like a copy for your reference.

I might add that now that I've tweaked your parsing, your variation handling (display, navigation) works really well. Bravo. See below.

Cheers


boleslavsky-goldenov moscow 1952

steinitz-zz commented 8 years ago

Oops, I never did show you the pgn. Here it is:

1. e4 c6 2. Nc3 d5 3. d4 dxe4 4. Nxe4 Nf6 5. Ng3 e5 6. Nf3 exd4 7. Nxd4 Bc5 8. Qe2+ Be7 9. Be3 Qd5 10. Qc4 Qe5 11. O-O-O Nd5 12. Nf3 Qe6 13. Bd2 b5 14. Qd4 Bf6 15. Qe4 O-O 16. Bd3 Qxe4 17. Nxe4 Be7 18. Rhe1 Na6 19. Nc3 Nac7 20. Nxd5 Nxd5 21. Be4 Be6 22. Nd4 Bc5 23. Nxe6 fxe6 24. f4 a5 25. g3 a4 26. a3 Rab8 27. c3 Rbd8 28. Bg2 Rd6 29. Bh3 Nc7 30. Be3 Bxe3+ 31. Rxe3 Rxd1+ 32. Kxd1 Kf7 33. Kc2 g6 34. Re5 Ke7 35. Bf1 Kd6 36. c4 bxc4 37. Bxc4 Nd5 38. h4 Rg8 39. Re4 Ra8 40. Be2 Ne7 41. Rd4+ Nd5 42. Re4 Ne7
    {Start of Shereshevsky Endgame}
43. Bc4 Nd5 44. Re5 Rb8 45. Bd3 Rg8 46. Kc1 c5 47. Bc4 Ra8 48. Re4 Ra7 49. Re2 Rb7 50. Kc2 Rc7 51. Bb5 Ra7 52. Rd2 Ke7 53. Re2 Kd6 54. Re4 Nb6 55. Bf1 Nd5 $2
    {But correct play is not much better - see variations}
    ( 55. ... Rb7 56. Bh3 Re7 57. Kd3 {Black is in an unusual zugswang - either Kd5 or Re8 allow a favourable breakthrough by f5 (Boleslavsky).} 57. ... Kd5
        ( 57. ... Re8 58. f5 )
    58. f5 )
56. Bh3 Nc7 57. Bg4 Ra8 58. Kc1 $3 Ra7 59. Bd1 Nb5 60. Bxa4 Nd4 61. Bd1 Nf5 62. Bg4 Rb7 63. Bxf5 exf5 64. Re3 Kd5 65. Kc2 c4 66. Re5+ Kd6
    {...Kd4 a4 would not have saved Black - Boleslavsky gave the following.}
    ( 66. ... Kd4 67. a4 Rb3
        ( 67. ... Ra7 68. b3 cxb3+ 69. Kxb3 )
        ( 67. ... Rb4 68. a5 Ra4 69. h5 gxh5
            ( 69. ... Ra2 70. h6 c3 71. Rb5 Kc4 72. Rb7 cxb2 73. Rxh7 Rxa5 74. Kxb2 Rb5+ 75. Kc2 Ra5 76. Rc7+ Kd4 77. Rg7 Ra2+ 78. Kb3 Rh2 79. Rxg6 )
        70. Rxf5 h4 71. gxh4 Ke4 72. Rc5 Kxf4 73. Kc3 Kg3 74. Rxc4 Rxa5 75. b4 )
    68. a5 Rxg3 69. a6 Rg1 70. Ra5 )
67. a4 Rb3 68. a5 Rxg3 69. a6 Kc7 70. Rb5 Rg1 71. Rb7+ Kc6 72. Rxh7 Ra1 73. Rg7 1-0

Here are the moves that caused problems:

and other similar black moves.

I hope this information is useful to you. Again, let me know if you'd like the tweaked peg code, for reference.

mliebelt commented 8 years ago

I have followed your "bug discussion" on https://github.com/pegjs/pegjs/issues/376, and do understand the reason here. I have to upgrade to version 0.9.0, when I want to use the strict mode of pegjs. I will do that in the future. Currently I don't have the time to do a real new release, which is necessary to have a consistent version again.

I will close that issue only when a new version is "released" which solves the problem with the strict mode. I will then invest some more time in the variations of PGN you touched in that issue.

steinitz-zz commented 8 years ago

Sounds sensible.

Below is a working version of pgn-rules.peg which is 0.9.0-compatible and solves pgn compatibility issues I encountered. I only made 5 or 6 changes to your excellent code. For any replacements, I kept your original versions in a comments marked 'SJS'. For new code I added // SJS. Feel free to use it. It works well for all pgn I've thrown at it.

Note that I also enhanced your pgn.js and pgnvjs.js to allow pgn game fragments which have a starting FEN in the header. Let me know if you want that.

{
    function makeInteger(o) {
        return parseInt(o.join(""), 10);
    }
}

pgn
  = pw:pgnStartWhite all:pgnBlack?
      { var arr = (all ? all : []); arr.unshift(pw);return arr; }
  / pb:pgnStartBlack all:pgnWhite?
    { var arr = (all ? all : []); arr.unshift(pb); return arr; }
  / whiteSpace?
    { return [[]]; }

pgnStartWhite
  = whiteSpace? pw:pgnWhite { return pw; }

pgnStartBlack
 = whiteSpace? cm:comment? whiteSpace? me:moveEllipse? all:pgnBlack // SJS = whiteSpace? cm:comment? whiteSpace? me:moveEllipse? all:pgnBlack
    { var last = all[0]; last.moveNumber = me; last.commentMove = cm; return all; }

pgnWhite
  = whiteSpace? cm:comment? whiteSpace? mn:moveNumber whiteSpace? cb:comment? whiteSpace?
    hm:halfMove whiteSpace? nag:nags? whiteSpace? ca:comment? whiteSpace? vari:variationWhite? all:pgnBlack?     // SJS hm:halfMove  nag:nags?  whiteSpace? ca:comment? whiteSpace? vari:variationWhite? all:pgnBlack?
    { var arr = (all ? all : []); var move = {}; move.turn = 'w'; move.moveNumber = mn;
      move.notation = hm; move.commentBefore = cb; move.commentAfter = ca; move.commentMove = cm;
      move.variations = (vari ? vari : []); move.nag = (nag ? nag : null); arr.unshift(move); return arr; }
  / endGame

pgnBlack
  = loneEllipse? whiteSpace? cb:comment? whiteSpace? me:moveEllipse? // SJS whiteSpace? cb:comment? whiteSpace? 
    hm:halfMove whiteSpace? nag:nags? whiteSpace? ca:comment? whiteSpace? vari:variationBlack? all:pgnWhite? // SJS hm:halfMove  nag:nags? whiteSpace? ca:comment? whiteSpace? vari:variationBlack? all:pgnWhite?
    { var arr = (all ? all : []); var move = {}; move.turn = 'b'; move.notation = hm;
      move.commentBefore = cb; move.commentAfter = ca;
      move.variations = (vari ? vari : []); arr.unshift(move); move.nag = (nag ? nag : null); return arr; }
  / endGame

endGame
  = "1:0" { return ["1:0"]; }
  / "0:1" { return ["0:1"]; }
  / "1-0" { return ["1:0"]; } // SJS
  / "0-1" { return ["0:1"]; } // SJS
  / "1/2-1/2"  { return ["1/2-1/2"]; }
  / "*"  { return ["*"]; }

comment
  = cl cm:[^}]+ cr { return cm.join("").trim(); }

cl = '{'

cr = '}'

variationWhite
  = pl vari:pgnWhite pr whiteSpace? all:variationWhite? whiteSpace? me:moveEllipse? 
    { var arr = (all ? all : []); arr.unshift(vari); return arr; }

variationBlack
  = pl vari:pgnStartBlack pr whiteSpace? all:variationBlack?
    { var arr = (all ? all : []); arr.unshift(vari); return arr; }

pl = '('

pr = ')'

moveNumber
    = num:integer"."? { return num; }

integer "integer"
    = digits:[0-9]+ { return makeInteger(digits); }

whiteSpace
    = [ \t\r\n]+ { return '';} // SJS = " "+ { return '';}

halfMove
  = fig:figure? & checkdisc disc:discriminator str:strike?
    col:column row:row pr:promotion? ch:check?
    { var hm = {};
      hm.fig = (fig ? fig : null);
      hm.disc =  (disc ? disc : null);
      hm.strike = (str ? str : null);
      hm.col = col;
      hm.row = row;
      hm.check = (ch ? ch : null);
      hm.promotion = pr;
      hm.notation = (fig ? fig : "") + (disc ? disc : "") + (str ? str : "") + col + row + (pr ? pr : "") + (ch ? ch : "");
      return hm; }
  / fig:figure? str:strike? col:column row:row pr:promotion? ch:check?
    { var hm = {};
      hm.fig = (fig ? fig : null);
      hm.strike = (str ? str : null);
      hm.col = col; hm.row = row;
      hm.check = (ch ? ch : null);
      hm.notation = (fig ? fig : "") + (str ? str : "") + col  + row + (pr ? pr : "") + (ch ? ch : "");
      hm.promotion = pr;
      return hm; }
  / 'O-O-O' { var hm = {}; hm.notation = 'O-O-O'; return  hm; }
  / 'O-O' { var hm = {}; hm.notation = 'O-O'; return  hm; }

check
  = '+'
  / '#'

promotion
  = '=' f:figure { return '=' + f; }

nags
  = nag:nag whiteSpace? nags:nags? { var arr = (nags ? nags : []); arr.unshift(nag); return arr; }

nag
  = '$' num:integer { return '$' + num; }
  / '!!' { return '$3'; }
  / '??' { return '$4'; }
  / '!?' { return '$5'; }
  / '?!' { return '$6'; }
  / '!' { return '$1'; }
  / '?' { return '$2'; }
  / '=' { return '$10'; }
  / '∞' { return '$13'; }
  / 'D' { return '$220'; }

discriminator
  = column
  / row

checkdisc
  = discriminator strike? column row

moveEllipse
  // SJS handle 55. ... Rb7 etc; was: = num:integer "."+ {return num;}
  = num:integer ['.'  \t\r\n]+ {return num;} 

loneEllipse = "."+ {return '';} // SJS allow for possible lone '...' before black move

figure
  = [RNBQK]

column
  = [a-h]

row
  = [1-8]

strike
  = 'x'
mliebelt commented 8 years ago

The new version I have just pushed should fix this issue, the new grammar (and version of PEG) is integrated.