nextcloud / cookbook

🍲 A library for all your recipes
https://apps.nextcloud.com/apps/cookbook
GNU Affero General Public License v3.0
523 stars 89 forks source link

Fix unicode fraction conversion #2498

Open christianlupus opened 1 week ago

christianlupus commented 1 week ago

Description When using unicode fractions, the web UI reports the ingredient to not be parsable but in fact it is parsable and correctly calculated.

Reproduction Steps to reproduce the behavior:

  1. Create or edit a recipe
  2. Add an ingredient with a unicode fraction, e.g. ¾
  3. Save and view the recipe
  4. Change the recipe yield to trigger recalculation

Expected behavior No warning is shown to the user

Actual behavior A warning sign is shown to the user indicating a problem with the calculation (see screenshots).

Screenshots grafik

After changing the yield amount: grafik

Browser Firefox

Versions Nextcloud server version: 30 Cookbook version: 0.11.1 Database system: MySQL


Maybe @Weissnix4711 or @j0hannesr0th, can you have a look at this? Ideally, I would say that the same Regex is used to check and to do the actual calculation to avoid such conflicts. That would mean some restructuring but should be managable.

j0hannesr0th commented 1 week ago

Hi @christianlupus I'll have a look at this one.

Do you want to support the common Unicode fractions or custom Unicode fractions like ⁷⁷⁄₇₈ too?

Weissnix4711 commented 1 week ago

Uhh, maybe I fucked the regex

christianlupus commented 1 week ago

In the PR there was a discussion to mainly look for x/2, x/4, and x/16 as far as I remember.

I suggest you two align a bit to avoid duplicate work.

christianlupus commented 1 week ago

Ahh and one more point: I want to push out a release tomorrow. On Saturday there is a NC server release and we need a new cookbook version ready as otherwise these will complain.

I am okay with delaying this until after the release. Just be warned that I am not going to be able to do much about this in the next few hours. :wink:

Weissnix4711 commented 1 week ago

Yeah ideally we should probably use the same regex, but the easy solution is this:

diff --git a/src/js/yieldCalculator.js b/src/js/yieldCalculator.js
index bfe2da97..47bf75d3 100644
--- a/src/js/yieldCalculator.js
+++ b/src/js/yieldCalculator.js
@@ -11,7 +11,7 @@ function isValidIngredientSyntax(ingredient) {
         It may optionally have a unit but must be proceeded by a single whitespace and further text.
     */
     const ingredientSyntaxRegExp =
-        /^(?:(?:\d+\s)?(?:\d+\/\d+|\p{No})|\d+(?:\.\d+)?)[a-zA-z]*\s.*$/;
+        /^(?:(?:\d+\s)?(?:\d+\/\d+|\p{No})|\d+(?:\.\d+)?)[a-zA-z]*\s.*$/u;

     /*
         The ingredientMultipleSeperatorsRegExp is used to check whether the string contains

I missed the u flag.

christianlupus commented 1 week ago

Feel free to give it a go from my side. Ideally there is some error handling involved if we update the regex (for whatever reason), this is not breaking again.