# HG changeset patch # User Mike Becker # Date 1788361916 -7200 # Node ID dc4f34426c669d10cf30a5b63771ca6d8c856ef8 # Parent e9c3d3e89465d84e7d6ec330b0161c97b6e7d147 improve error reporting in PGN parser - resolves #958 diff -r e9c3d3e89465 -r dc4f34426c66 src/chess/pgn.c --- a/src/chess/pgn.c Tue Sep 01 17:47:07 2026 +0200 +++ b/src/chess/pgn.c Wed Sep 02 17:11:56 2026 +0200 @@ -41,24 +41,17 @@ "Missing closing brace '}' for comment.", "Tags must be enclosed in square brackets: '[Key \"Value\"]'.", "Move numbers must be terminated with a dot (e.g. '13.' - not '13').", - "Move is syntactically incorrect.", - "Move is not valid according to chess rules.", "Result syntax is incorrect. Expected 1-0, 0-1, 1/2-1/2, or *.", "Result at end of move text does not match result in tag roster.", "Unexpected end of data.", + "Invalid move.", }; const char* pgn_error_str(int code) { return pgn_error_strings[code]; } -size_t pgn_error_pos; - -size_t pgn_error_position(void) { - return pgn_error_pos; -} - -static const char *pgn_result(const GameState *gamestate) { +static const char *pgn_game_result(const GameState *gamestate) { if (is_game_drawn(gamestate)) { return "1/2-1/2"; } else if (gamestate->wresign) { @@ -76,9 +69,28 @@ } } -#define return(code) pgn_error_pos = pgndata - pgndata0; return code +static pgn_result create_pgn_result( + const char *datastart, const char *datacur, + enum pgn_error_code code, unsigned move_half_number) { + return (pgn_result) { + .position = (size_t)(datacur - datastart), + .code = code, + .move_half_number = move_half_number + }; +} -int parse_pgn(const char *pgndata, GameState *gamestate) { +static pgn_result create_pgn_move_error( + const char *datastart, const char *datacur, + int move_error_code, unsigned move_half_number) { + return (pgn_result) { + .position = (size_t)(datacur - datastart), + .code = pgn_error_move, + .move_error = move_error_code, + .move_half_number = move_half_number + }; +} + +pgn_result parse_pgn(const char *pgndata, GameState *gamestate) { const char * const pgndata0 = pgndata; int i; char c; @@ -95,7 +107,8 @@ break; } if (c != '[') { - return(pgn_error_missing_bracket); + return create_pgn_result(pgndata0, pgndata, + pgn_error_missing_bracket, 0); } while (isspace(c = *(pgndata++))); i = 0; @@ -105,18 +118,21 @@ tagkey[i] = '\0'; while (isspace(c = *(pgndata++))); if (c != '"') { - return(pgn_error_missing_quote); + return create_pgn_result(pgndata0, pgndata, + pgn_error_missing_quote, 0); } i = 0; while ((c = *(pgndata++)) != '"') { if (c == '\n' || c == 0) { - return(pgn_error_missing_quote); + return create_pgn_result(pgndata0, pgndata, + pgn_error_missing_quote, 0); } tagvalue[i++] = c; } tagvalue[i] = '\0'; if (*(pgndata++) != ']') { - return(pgn_error_missing_bracket); + return create_pgn_result(pgndata0, pgndata, + pgn_error_missing_bracket, 0); } // TODO: read clock info @@ -128,13 +144,15 @@ /* read moves */ if (*(pgndata++) != '.') { - return(pgn_error_missing_dot); + return create_pgn_result(pgndata0, pgndata, + pgn_error_missing_dot, 1); } char movestr[10]; Move move; Color curcol = WHITE; bool movetext_ends_with_result = false; + unsigned move_half_number = 1; while (true) { /* move */ @@ -143,17 +161,18 @@ do { movestr[i++] = c; if (i >= 10) { - return(pgn_error_move_syntax); + return create_pgn_move_error(pgndata0, pgndata, + INVALID_MOVE_SYNTAX, move_half_number); } } while (!isspace(c = *(pgndata++))); movestr[i] = '\0'; - if (eval_move_strict(gamestate, movestr, curcol, &move) - != VALID_MOVE_SYNTAX) { - return(pgn_error_move_syntax); + int move_result = eval_move_strict(gamestate, movestr, curcol, &move); + if (!move_result) { + move_result = validate_move(gamestate, &move); } - int move_validate_result = validate_move(gamestate, &move); - if (move_validate_result != VALID_MOVE_SEMANTICS) { - return(pgn_error_move_semantics); + if (move_result) { + return create_pgn_move_error(pgndata0, pgndata, + move_result, move_half_number); } format_move(gamestate, &move); apply_move(gamestate, &move); @@ -168,7 +187,8 @@ c = *(pgndata++); } while (c != '}' && c != 0); if (c == 0) { - return(pgn_error_missing_brace); + return create_pgn_result(pgndata0, pgndata, + pgn_error_missing_brace, move_half_number); } /* skip spaces */ while (isspace(c = *(pgndata++))); @@ -188,7 +208,8 @@ } else if (c == '0') { gamestate->bresign = true; } else { - return(pgn_error_result_syntax); + return create_pgn_result(pgndata0, pgndata, + pgn_error_result_syntax, 0); } } movetext_ends_with_result = true; @@ -211,25 +232,28 @@ if (curcol == BLACK) { while (isdigit(c = *(pgndata++))); if (c != '.') { - return(pgn_error_missing_dot); + return create_pgn_result(pgndata0, pgndata, + pgn_error_missing_dot, move_half_number + 1); } } curcol = opponent_color(curcol); + move_half_number++; } /* sanity check result - if it was specified */ if (movetext_ends_with_result && result[0]) { - if (strncmp(result, pgn_result(gamestate), 8) != 0) { - return(pgn_error_result_mismatch); + if (strncmp(result, pgn_game_result(gamestate), 8) != 0) { + return create_pgn_result(pgndata0, pgndata, + pgn_error_result_mismatch, 0); } } - return(pgn_no_error); + return create_pgn_result(pgndata0, pgndata, pgn_no_error, 0); } #undef return -int read_pgn(FILE *stream, GameState *gamestate) { +pgn_result read_pgn(FILE *stream, GameState *gamestate) { fseek(stream, 0, SEEK_END); size_t size = ftell(stream); fseek(stream, 0, SEEK_SET); @@ -239,12 +263,14 @@ do { size_t r = fread(data + read, 1, size - read, stream); if (r == 0) { - pgn_error_pos = read; - return pgn_error_unexpected_eof; + return (pgn_result) { + .position = read, + .code = pgn_error_unexpected_eof + }; } read += r; } while (read < size); - int result = parse_pgn(data, gamestate); + pgn_result result = parse_pgn(data, gamestate); free(data); return result; } @@ -328,7 +354,7 @@ resp += sprintf(resp, "[Round \"%s\"]\n", "-"); resp += sprintf(resp, "[White \"%s\"]\n", pgn_player_name(gamestate, WHITE)); resp += sprintf(resp, "[Black \"%s\"]\n", pgn_player_name(gamestate, BLACK)); - const char *gameresult = pgn_result(gamestate); + const char *gameresult = pgn_game_result(gamestate); resp += sprintf(resp, "[Result \"%s\"]\n\n", gameresult); // TODO: add optional clock info diff -r e9c3d3e89465 -r dc4f34426c66 src/chess/pgn.h --- a/src/chess/pgn.h Tue Sep 01 17:47:07 2026 +0200 +++ b/src/chess/pgn.h Wed Sep 02 17:11:56 2026 +0200 @@ -38,29 +38,36 @@ extern "C" { #endif -enum { +enum pgn_error_code { pgn_no_error = 0, pgn_error_missing_quote, pgn_error_missing_bracket, pgn_error_missing_brace, pgn_error_missing_dot, - pgn_error_move_syntax, - pgn_error_move_semantics, pgn_error_result_syntax, pgn_error_result_mismatch, pgn_error_unexpected_eof, + pgn_error_move, }; -int read_pgn(FILE *stream, GameState *gamestate); +typedef struct { + size_t position; + enum pgn_error_code code; + int move_error; // TODO: should probably also be an enum eventually + unsigned move_half_number; +} pgn_result; + + +pgn_result read_pgn(FILE *stream, GameState *gamestate); int write_pgn(FILE* stream, const GameState *gamestate, bool export_comments); -int parse_pgn(const char *data, GameState *gamestate); +pgn_result parse_pgn(const char *data, GameState *gamestate); char *create_pgn(const GameState *gamestate, bool export_comments); +// TODO: this function looks a bit misplaced const char *pgn_player_name(const GameState *gamestate, Color color); const char* pgn_error_str(int code); -size_t pgn_error_position(void); #ifdef __cplusplus } diff -r e9c3d3e89465 -r dc4f34426c66 src/main.c --- a/src/main.c Tue Sep 01 17:47:07 2026 +0200 +++ b/src/main.c Wed Sep 02 17:11:56 2026 +0200 @@ -1289,11 +1289,20 @@ if (settings.continuepgn) { FILE *pgnfile = fopen(settings.continuepgn, "r"); if (pgnfile) { - int result = read_pgn(pgnfile, &gamestate); + pgn_result result = read_pgn(pgnfile, &gamestate); fclose(pgnfile); - if (result) { - printw("Invalid PGN file content at position %zu:\n%s\n", - pgn_error_position(), pgn_error_str(result)); + if (result.code) { + printw("Invalid PGN content at position %zu", result.position); + if (result.move_half_number) { + unsigned hm = result.move_half_number - 1; + printw(" (move %u, %s)", 1+hm/2, hm%2 ? "black":"white"); + } + printw(":\n%s", pgn_error_str(result.code)); + if (result.code == pgn_error_move) { + addch(' '); + eval_move_failed_msg(result.move_error); + } + printw("\n"); exitcode = EXIT_FAILURE; } /* clear resignation and remis flags to allow game continuation */