cooklang / cooklang-swift

Cooklang parser implementation in Swift
https://cooklang.org
MIT License
38 stars 3 forks source link

Equipment tags don't work for equipment starting with a number #5

Closed theunraveler closed 3 years ago

theunraveler commented 3 years ago

Hello. I noticed that one of my cookware/equipment specifications in a recipe wasn't being recognized as equipment. It seems to happen with equipment that starts with a number. I was able to add a test case that demonstrates the bug, but don't know enough about language parsing to actually fix the issue. Here's a patch that adds the test case:

diff --git a/Tests/CookInSwiftTests/ParserTests.swift b/Tests/CookInSwiftTests/ParserTests.swift
index edd7e2d..a944bfc 100644
--- a/Tests/CookInSwiftTests/ParserTests.swift
+++ b/Tests/CookInSwiftTests/ParserTests.swift
@@ -388,6 +388,24 @@ class ParserTests: XCTestCase {

         XCTAssertEqual(result, node)
     }
+
+    func testEquipmentMultipleWordsWithLeadingNumber() {
+        let recipe =
+            """
+            Fry in #7-inch nonstick frying pan{ }
+            """
+
+        let result = try! Parser.parse(recipe) as! RecipeNode
+
+        let steps = [
+            StepNode(instructions: [
+                    DirectionNode("Fry in "),
+                    EquipmentNode(name: "7-inch nonstick frying pan")]),
+        ]
+        let node = RecipeNode(steps: steps)
+
+        XCTAssertEqual(result, node)
+    }

     func testTimerInteger() {
         let recipe =

Thanks!

theunraveler commented 3 years ago

Also, it seems like the same thing happens with ingredients. Here are test cases for both scenarios.

diff --git a/Tests/CookInSwiftTests/ParserTests.swift b/Tests/CookInSwiftTests/ParserTests.swift
index edd7e2d..41c7382 100644
--- a/Tests/CookInSwiftTests/ParserTests.swift
+++ b/Tests/CookInSwiftTests/ParserTests.swift
@@ -231,6 +231,24 @@ class ParserTests: XCTestCase {

         XCTAssertEqual(result, node)
     }
+
+    func testIngredientMultipleWordsWithLeadingNumber() {
+        let recipe =
+            """
+            Top with @1000 island dressing{ }
+            """
+
+        let result = try! Parser.parse(recipe) as! RecipeNode
+
+        let steps = [
+            StepNode(instructions: [
+                    DirectionNode("Top with "),
+                    IngredientNode(name: "1000 island dressing", amount: AmountNode(quantity: ConstantNode.integer(1)))]),
+        ]
+        let node = RecipeNode(steps: steps)
+
+        XCTAssertEqual(result, node)
+    }

     func testIngridentWithoutStopper() {
         let recipe =
@@ -388,6 +406,24 @@ class ParserTests: XCTestCase {

         XCTAssertEqual(result, node)
     }
+
+    func testEquipmentMultipleWordsWithLeadingNumber() {
+        let recipe =
+            """
+            Fry in #7-inch nonstick frying pan{ }
+            """
+
+        let result = try! Parser.parse(recipe) as! RecipeNode
+
+        let steps = [
+            StepNode(instructions: [
+                    DirectionNode("Fry in "),
+                    EquipmentNode(name: "7-inch nonstick frying pan")]),
+        ]
+        let node = RecipeNode(steps: steps)
+
+        XCTAssertEqual(result, node)
+    }

     func testTimerInteger() {
         let recipe =

I think I see where this is handled at https://github.com/cooklang/CookInSwift/blob/main/Sources/CookInSwift/Parser/Parser.swift#L126. I think I can fix it, but the spec isn't sufficiently clear as to what values are allowed in a cookware or ingredient item, so I'm not sure what types of tokens to include in the definition, particularly with edge cases. For example, it seems like #@oven should not be a valid cookware node. Nor should #{oven. So should that line just be looking for .constant(.string) and .constant(.integer) or something?

Thanks!

dubadub commented 3 years ago

Thanks for test cases, this was really helpful! Issue fixed, changes are in CLI since v0.0.13. Meanwhile I added EBNF, so it's less obscure what we can expect in different situations.