cooklang / cooklang-ts

Lightweight Cooklang parser implemented in TypeScript
https://cooklang.github.io/cooklang-ts/
MIT License
75 stars 7 forks source link

[BUG] Problem(s) with step number in AST #18

Closed lucharo closed 11 months ago

lucharo commented 1 year ago

Describe the bug A while ago I requested expanding the AST to include the step number(s) for the ingredients in a recipe. I've now realized that the cooklang specification suggests only mentioning each ingredient once:

 If you’re writing a recipe, only @mention each ingredient once.

After the first time, you can simply refer to the ingredient by name and relative quantity (e.g. “then put half the remaining bacon in the oven to crispen.”).

so because of this the step element in the AST doesn't properly work when an ingredient is used in several steps, e.g. Chop cabbage in step 1, put potatoes in the oven in step 2, combine potatoes and cabbage in step 3. With the cooklang suggested specification cabbage would only list step 1 and potatoes would only list step 2 rather than listing steps 1,3 and steps 1,2 respectively.

I realize that I can omit the cooklang spec suggestion and just add @ingredient{0} at every step but that would add ingredient 0, 0, 0 to the ingredients list when parsed.

Note while writing out the issue: I'm getting every step number to be wrong by 1 (step listed as 3 instead of 2 when ingredient listen in 2nd step)

To Reproduce I have a recipe that repeats ingredients in several steps (tomatoes) as you can tell tomatoes only list step number 1 instead of all the steps they're needed in

import { Recipe, Parser, getImageURL } from '@cooklang/cooklang-ts';

const source = `
>> name: Stuffed Tomatoes with Rice
>> servings: 4

Preheat the #oven{} to ~{180°C}.

Cut the tops off of @tomatoes{4 large} and scoop out the insides to create a 'shell'. Keep the insides.

In a #pan{}, sauté @onions{1 chopped} and @garlic{2 cloves minced} in @olive oil{2 tbsp} until translucent.

Add the insides of the tomatoes (chopped) to the pan and cook for ~{5 minutes}.

Add @uncooked rice{1 cup} to the pan, along with @water{2 cups}, @salt{}, and @pepper{}. Allow the mixture to simmer until the rice is cooked, approximately ~{18 minutes}.

In the meantime, place the tomato 'shells' in a #baking dish{} and drizzle with a bit of olive oil, then bake for ~{10 minutes}.

Once the rice is cooked, add chopped @fresh basil{2 tbsp} and @grated Parmesan cheese{1/4 cup} to the pan and stir to combine.

Fill the pre-baked tomato shells with the rice mixture.

Return the stuffed tomatoes to the oven and bake for an additional ~{15 minutes}.

Garnish with more fresh basil and serve hot.

`;

console.log(
    new Parser({
        includeStepNumber: true
    })
        .parse(source)
        .ingredients
    );

I run it with ts-node and I get:

[
  {
    type: 'ingredient',
    name: 'tomatoes',
    quantity: '4 large',
    units: '',
    step: 3
  },
...
]

Expected behavior

Actually just noticed that I'm getting the wrong step number (3) instead of 2

[
  {
    type: 'ingredient',
    name: 'tomatoes',
    quantity: '4 large',
    units: '',
    steps: [2, 4, 6, 7]
  },
...
]
ThatTSGuy commented 1 year ago

I don't think its a good idea to include non marked-down ingredients when found, in the step field of an ingredient node. It's my firm belief that unless it is non-text node (@ingredient, #cookware, ect) it should not effect the nodes in each list of ingredients, cookware, ect. This is because each node, lets say the in ingredient array, represents a singe mention in the recipe. I'm open to suggestions, and maybe I'm misunderstanding your problem. (The wrong step number is a separate issue I will fix)

ThatTSGuy commented 11 months ago

Fixed issue in 4de62a882bbf948095a94b5ed4f006dee02b9a73.