fix: Provide better error messages when parsing PPN metadata

closes #111

DESCRIPTION

@@ -2,10 +2,10 @@ Encoding: UTF-8
 Package: ppgames
 Type: Package
 Title: Piecepack Games
-Version: 0.6.2-12
+Version: 0.6.2-13
 Author: Trevor L Davis. 
 Maintainer: Trevor L Davis <trevor.l.davis@gmail.com>
-Description: This is an R package designed to make piecepack game diagrams.
+Description: Functions to generate piecepack game graphics, rulesets, and books as well as functions to parse "Portable Piecepack Notation" files and a "Fuji-san" solver.
 License: CC BY-SA 4.0
 URL: https://github.com/piecepackr/ppgames
 BugReports: https://github.com/piecepackr/ppgames/issues

R/ppn.R

@@ -34,7 +34,10 @@ write_ppn <- function(games = list(), file = "") {
 # @param file Filename
 # @return A list, each element is a character vector containing the text of the PPN games within that file
 parse_ppn_file <- function(file) {
-    text <- readLines(file)
+    text <- readlines_ppn(file)
+    parse_contents(text)
+}
+parse_contents <- function(text) {
     game_starts <- grep("^-{3}", text)
     if (length(game_starts) == 0L || game_starts[1L] != 1L) {
         game_starts <- c(1L, game_starts)
@@ -46,6 +49,17 @@ parse_ppn_file <- function(file) {
     }
     contents
 }
+readlines_ppn <- function(file) {
+    tryCatch({
+        readLines(file)
+    }, warning = function(w) {
+        abort(w$message, class = "readlines_ppn", parent = w)
+    }, error = function(e) {
+        msg <- paste("Couldn't read the file", file)
+        msg <- c(msg, i = e$message)
+        abort(msg, class = "readlines_ppn", parent = e)
+    })
+}
 
 # Parse ppn game
 #
@@ -54,12 +68,21 @@ parse_ppn_file <- function(file) {
 # @return A list with a named list element named \code{Metadata}
 #         and character vector element named \code{Movetext}
 parse_ppn_game <- function(text, parse = TRUE) {
+    l <- extract_metadata_movetext(text)
+    if (parse) {
+        parse_movetext(l$movetext, l$metadata)
+    } else {
+        list(metadata = l$metadata, movetext = l$movetext)
+    }
+}
+
+extract_metadata_movetext <- function(text) {
     yaml_end <- grep("^\\.{3}", text)
     if (length(yaml_end) == 0L) {
         yaml_end <- grep("^[[:blank:]]+|^$", text)
     }
     if (length(yaml_end) > 0L) {
-        metadata <- yaml::yaml.load(text[1L:yaml_end[1L]])
+        metadata <- yaml_load(text[1L:yaml_end[1L]])
         if (yaml_end[1]<length(text)) {
             movetext <- text[(yaml_end[1L]+1L):length(text)]
         } else {
@@ -69,11 +92,27 @@ parse_ppn_game <- function(text, parse = TRUE) {
         metadata <- list()
         movetext <- text
     }
-    if (parse) {
-        parse_movetext(movetext, metadata)
-    } else {
-        list(metadata = metadata, movetext = movetext)
+    if (is.null(metadata))
+        metadata <- list()
+    if (!is.list(metadata)) {
+        msg <- paste0("The PPN metadata does not appear to be a YAML dictionary:\n",
+                     yaml::as.yaml(metadata))
+        abort(msg, class = "extract_metadata")
     }
+    list(metadata = metadata, movetext = movetext)
+}
+
+yaml_load <- function(text) {
+    tryCatch(yaml::yaml.load(text),
+             warning = function(w) {
+                abort(w$message, class = "yaml_load")
+             },
+             error = function(e) {
+                text <- paste(paste(" ", text), collapse = "\n")
+                msg <- c("YAML parsing error:", i = e$message,
+                         i = paste("Failed to parse the following YAML text:\n", text))
+                abort(msg, class = "yaml_load")
+             })
 }
 
 parse_movetext <- function(movetext, metadata) {

tests/testthat/test_ppn.R

@@ -93,6 +93,10 @@ test_that("parsing ppn files works as expected", {
     comments <- "---\n...\n{comment,with,commas}"
     g <- read_ppn(textConnection(comments))[[1]]
     expect_equal(g$comments[[2]], "comment,with,commas")
+
+    ppn_bad_metadata <- "---\nttt\n...\n"
+    expect_error(read_ppn(textConnection(ppn_bad_metadata)),
+                 "The PPN metadata does not appear to be a YAML dictionary")
 })
 
 test_that("parsing simplified piece notation works as expected", {