From 6dff8528f5546b7831a982da5a9ef125afe2f2a4 Mon Sep 17 00:00:00 2001 From: Martin Bodin Date: Mon, 4 Dec 2023 17:12:00 +0100 Subject: [PATCH 1/4] This should fix #326. --- syntax/attribute_value.ml | 31 +++++++++++++++++++++++++------ syntax/reflect/reflect.ml | 2 +- test/test_jsx.re | 5 +++++ test/test_ppx.ml | 4 ++++ 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/syntax/attribute_value.ml b/syntax/attribute_value.ml index 4630d6e11..a38710c72 100644 --- a/syntax/attribute_value.ml +++ b/syntax/attribute_value.ml @@ -74,11 +74,11 @@ let list |> Common.list loc |> fun e -> Some e -let spaces = list (Re_str.regexp " +") "space" -let commas = list (Re_str.regexp " *, *") "comma" -let semicolons = list (Re_str.regexp " *; *") "semicolon" +let spaces = list (Re_str.regexp "[ \t\r\f]+") "space" +let commas = list (Re_str.regexp "[ \t\r\f]*,[ \t\r\f]*") "comma" +let semicolons = list (Re_str.regexp "[ \t\r\f]*;[ \t\r\f]*") "semicolon" -let spaces_or_commas_regexp = Re_str.regexp "\\( *, *\\)\\| +" +let spaces_or_commas_regexp = Re_str.regexp "\\([ \t\r\f]*,[ \t\r\f]*\\)\\|[ \t\r\f]+" let spaces_or_commas_ = exp_list spaces_or_commas_regexp "space- or comma" let spaces_or_commas = list spaces_or_commas_regexp "space- or comma" @@ -347,8 +347,8 @@ let offset = else Some [%expr `Number [%e n]] end [@metaloc loc] -let transform = - let regexp = Re_str.regexp "\\([^(]+\\)(\\([^)]*\\))" in +let transform_item = + let regexp = Re_str.regexp "\\([a-zA-Z]+\\)[ \t\r\f]*(\\([^)]*\\))" in fun ?separated_by:_ ?default:_ loc name s -> if not @@ does_match regexp s then @@ -408,6 +408,25 @@ let transform = Some e +let rec transform = + let regexp_wsp = Re_str.regexp "[ \t\r\f]*" in + let regexp = Re_str.regexp "[ \t\r\f]*\\([a-zA-Z]+[ \t\r\f]*([^)]*)\\)\\(.*\\)" in + + fun ?separated_by:_ ?default:_ loc name s -> + if does_match regexp_wsp s then + Some [%expr []] + else if does_match regexp_wsp s then + begin + let item = Re_str.matched_group 1 s in + let rest = Re_str.matched_group 2 s in + Option.bind (transform_item ~separated_by ~default loc name item) (fun item -> + Option.bind (transform ~separated_by ~default loc name rest) (fun l -> + Some (item :: l))) + end + else + Common.error loc "Value of %s is not a list of SVG transform" name + [@metaloc loc] + (* String-like. *) diff --git a/syntax/reflect/reflect.ml b/syntax/reflect/reflect.ml index dafd1975b..d8abebf60 100644 --- a/syntax/reflect/reflect.ml +++ b/syntax/reflect/reflect.ml @@ -200,7 +200,7 @@ let rec to_attribute_parser lang name ~loc = function [%expr spaces_or_commas svg_length] | [[%type: transforms]] -> - [%expr spaces_or_commas transform] + [%expr transform] | [[%type: paint]] -> [%expr paint] diff --git a/test/test_jsx.re b/test/test_jsx.re index 18a5d4a7c..3f811a9b8 100644 --- a/test/test_jsx.re +++ b/test/test_jsx.re @@ -341,6 +341,11 @@ let svg = ( [], [path(~a=[a_fill_rule(`Evenodd)], [])], ), + ( + "transform with random spacing", + [], + [g(~a:[a_transform([`Translate((200., Some(200.))), `Rotate((1., None)), `Matrix((-0., 1, 0.1, 0., 1e-5, 1e5)), `Scale((1., Some 0.)), `SkewY(-0.)])], [])], + ), ], ), ); diff --git a/test/test_ppx.ml b/test/test_ppx.ml index b521aada4..0d45c154c 100644 --- a/test/test_ppx.ml +++ b/test/test_ppx.ml @@ -424,6 +424,10 @@ let svg = "svg", SvgTests.make Svg.[ [[%svg ""]], [path ~a:[a_fill_rule `Evenodd] []] ; + "transform with random spacing", + [[%svg ""]], + [g ~a:[a_transform [`Translate (200., Some 200.); `Rotate (1., None); `Matrix (-0., 1, 0.1, 0., 1e-5, 1e5); `Scale (1., Some 0.); `SkewY (-0.)]] []] ; + ] let svg_element_names = "svg element names", SvgTests.make Svg.[ From 5796920e5bdbfc0c478ad4dd6ce92b83be049783 Mon Sep 17 00:00:00 2001 From: Martin Bodin Date: Mon, 4 Dec 2023 17:23:24 +0100 Subject: [PATCH 2/4] What they write \f in the spec actually means \n. --- syntax/attribute_value.ml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/syntax/attribute_value.ml b/syntax/attribute_value.ml index a38710c72..0ae46c003 100644 --- a/syntax/attribute_value.ml +++ b/syntax/attribute_value.ml @@ -74,11 +74,11 @@ let list |> Common.list loc |> fun e -> Some e -let spaces = list (Re_str.regexp "[ \t\r\f]+") "space" -let commas = list (Re_str.regexp "[ \t\r\f]*,[ \t\r\f]*") "comma" -let semicolons = list (Re_str.regexp "[ \t\r\f]*;[ \t\r\f]*") "semicolon" +let spaces = list (Re_str.regexp "[ \t\r\n]+") "space" +let commas = list (Re_str.regexp "[ \t\r\n]*,[ \t\r\n]*") "comma" +let semicolons = list (Re_str.regexp "[ \t\r\n]*;[ \t\r\n]*") "semicolon" -let spaces_or_commas_regexp = Re_str.regexp "\\([ \t\r\f]*,[ \t\r\f]*\\)\\|[ \t\r\f]+" +let spaces_or_commas_regexp = Re_str.regexp "\\([ \t\r\n]*,[ \t\r\n]*\\)\\|[ \t\r\n]+" let spaces_or_commas_ = exp_list spaces_or_commas_regexp "space- or comma" let spaces_or_commas = list spaces_or_commas_regexp "space- or comma" @@ -348,7 +348,7 @@ let offset = end [@metaloc loc] let transform_item = - let regexp = Re_str.regexp "\\([a-zA-Z]+\\)[ \t\r\f]*(\\([^)]*\\))" in + let regexp = Re_str.regexp "\\([a-zA-Z]+\\)[ \t\r\n]*(\\([^)]*\\))" in fun ?separated_by:_ ?default:_ loc name s -> if not @@ does_match regexp s then @@ -409,8 +409,8 @@ let transform_item = Some e let rec transform = - let regexp_wsp = Re_str.regexp "[ \t\r\f]*" in - let regexp = Re_str.regexp "[ \t\r\f]*\\([a-zA-Z]+[ \t\r\f]*([^)]*)\\)\\(.*\\)" in + let regexp_wsp = Re_str.regexp "[ \t\r\n]*" in + let regexp = Re_str.regexp "[ \t\r\n]*\\([a-zA-Z]+[ \t\r\n]*([^)]*)\\)\\(.*\\)" in fun ?separated_by:_ ?default:_ loc name s -> if does_match regexp_wsp s then @@ -419,9 +419,9 @@ let rec transform = begin let item = Re_str.matched_group 1 s in let rest = Re_str.matched_group 2 s in - Option.bind (transform_item ~separated_by ~default loc name item) (fun item -> - Option.bind (transform ~separated_by ~default loc name rest) (fun l -> - Some (item :: l))) + Option.bind (transform_item loc name item) (fun item -> + Option.bind (transform loc name rest) (fun l -> + Some [%expr [%e item] :: [%e l]])) end else Common.error loc "Value of %s is not a list of SVG transform" name From dd2297aea80b14ecb3c53d945d36d986109cd35a Mon Sep 17 00:00:00 2001 From: Martin Bodin Date: Mon, 4 Dec 2023 17:36:06 +0100 Subject: [PATCH 3/4] Fixing mistakes. --- syntax/attribute_value.ml | 2 +- test/test_jsx.re | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/syntax/attribute_value.ml b/syntax/attribute_value.ml index 0ae46c003..4abef0d83 100644 --- a/syntax/attribute_value.ml +++ b/syntax/attribute_value.ml @@ -415,7 +415,7 @@ let rec transform = fun ?separated_by:_ ?default:_ loc name s -> if does_match regexp_wsp s then Some [%expr []] - else if does_match regexp_wsp s then + else if does_match regexp s then begin let item = Re_str.matched_group 1 s in let rest = Re_str.matched_group 2 s in diff --git a/test/test_jsx.re b/test/test_jsx.re index 3f811a9b8..b5c68f006 100644 --- a/test/test_jsx.re +++ b/test/test_jsx.re @@ -344,7 +344,7 @@ let svg = ( ( "transform with random spacing", [], - [g(~a:[a_transform([`Translate((200., Some(200.))), `Rotate((1., None)), `Matrix((-0., 1, 0.1, 0., 1e-5, 1e5)), `Scale((1., Some 0.)), `SkewY(-0.)])], [])], + [g(~a=[a_transform([`Translate((200., Some(200.))), `Rotate((1., None)), `Matrix((-0., 1, 0.1, 0., 1e-5, 1e5)), `Scale((1., Some(0.))), `SkewY(-0.)])], [])], ), ], ), From 25cdd2772e0be4e89c0d0eecd9a59c3c13b7bacc Mon Sep 17 00:00:00 2001 From: Martin Bodin Date: Tue, 5 Dec 2023 10:33:12 +0100 Subject: [PATCH 4/4] Removing skewY from the test as it should be treated as a separate issue. --- test/test_jsx.re | 4 ++-- test/test_ppx.ml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_jsx.re b/test/test_jsx.re index b5c68f006..1827b7e5b 100644 --- a/test/test_jsx.re +++ b/test/test_jsx.re @@ -343,8 +343,8 @@ let svg = ( ), ( "transform with random spacing", - [], - [g(~a=[a_transform([`Translate((200., Some(200.))), `Rotate((1., None)), `Matrix((-0., 1, 0.1, 0., 1e-5, 1e5)), `Scale((1., Some(0.))), `SkewY(-0.)])], [])], + [], + [g(~a=[a_transform([`Translate((200., Some(200.))), `Matrix((-0., 1., 0.1, 0., 1e-5, 1e5)), `Scale((-1., Some(0.)))])], [])], ), ], ), diff --git a/test/test_ppx.ml b/test/test_ppx.ml index 0d45c154c..4fa3d06c0 100644 --- a/test/test_ppx.ml +++ b/test/test_ppx.ml @@ -425,8 +425,8 @@ let svg = "svg", SvgTests.make Svg.[ [path ~a:[a_fill_rule `Evenodd] []] ; "transform with random spacing", - [[%svg ""]], - [g ~a:[a_transform [`Translate (200., Some 200.); `Rotate (1., None); `Matrix (-0., 1, 0.1, 0., 1e-5, 1e5); `Scale (1., Some 0.); `SkewY (-0.)]] []] ; + [[%svg ""]], + [g ~a:[a_transform [`Translate (200., Some 200.); `Matrix (-0., 1., 0.1, 0., 1e-5, 1e5); `Scale (-1., Some 0.)]] []] ; ]