Engelberg / instaparse

Eclipse Public License 1.0
2.74k stars 149 forks source link

Error in instaparse.core/parser: Add :start-production in :else clause #212

Closed klausharbo closed 2 years ago

klausharbo commented 2 years ago

I found a minor error in the behaviour of instaparse.core/parser with respect to assignment of start production. The issue can be seen from this example:

user> (let [f  "test.bnf"
            g1 (io/file f)
            g2 (slurp g1)
            pars #(instaparse.core/parser % :start :other)
            p1 (pars g1)
            p2 (pars g2)]
        {:g1 g1
         :g2 g2
         :start1 (:start-production p1)
         :start2 (:start-production p2)
         :same? (= p1 p2)
         :otherwise-same? (= (dissoc p1 :start-production) (dissoc p2 :start-production))})
{:g1 #object[java.io.File 0x28920de7 "test.bnf"],
 :g2 "rule = \"xxx\"\n",
 :start1 :rule,
 :start2 :other,
 :same? false,
 :otherwise-same? true}
user> 

in which the start production of the grammar rule = "xxx" (in file test.bnf) depends on whether the grammar is provided to parser as either a string or a file/resource object: In the above example start production is :rule in the former and :other in the latter.

The PR provides a straightforward fix.

-Klaus.

Engelberg commented 2 years ago

Thanks for the error report and the pull request. Much appreciated. Since I revisit instaparse sporadically, I want to incorporate one or two other pull requests as I put out an update. This one is straightforward, but it will take me a few days to evaluate the others and do it all at once. If I haven't incorporated this within a week, please send me a gentle reminder.

klausharbo commented 2 years ago

Hereby gently reminding... :-)

Engelberg commented 2 years ago

OK, I didn't really get anything else put together, but rather than delay further, I went ahead and pushed this change out as v1.4.11. Thanks again for the fix.