JavaScript: fix string_builder.reverse, cleanup (#300)

6 files changed, 65 insertions, 28 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 106b4e5..6d2ca82 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,9 +5,10 @@
 - The `float` module gains the `divide` function.
 - The `int` module gains the `divide`, `power`, and `square_root` functions.
 - The `string` module gains the `first`, `last`, `capitalise` and `inspect` functions.
-- The `string` module gains the `first`, `last`, and `capitalise` functions.
+- Fixed a bug where `string_builder.reverse` would break utf8 strings on target JavaScript.
 - Fixed a bug where `string.reverse` would break utf8 strings on target JavaScript.
 - Fixed a bug where `string.slice` would break utf8 strings on target JavaScript.
+- The `string_builder` module loses the `from_float` function. Use `float.to_string` instead.
 - Fixed the `int.power` and `float.power` functions by properly handling error cases.
 - The grapheme iterator used by `string.graphemes` is now locale independent on target JavaScript.
 
diff --git a/src/gleam/float.gleam b/src/gleam/float.gleam
index e72eb0d..992499a 100644
--- a/src/gleam/float.gleam
+++ b/src/gleam/float.gleam
@@ -1,5 +1,4 @@
 import gleam/order.{Order}
-import gleam/string_builder
 
 /// Attempts to parse a string as a `Float`, returning `Error(Nil)` if it was not
 /// possible.
@@ -36,9 +35,17 @@ if javascript {
 /// ```
 ///
 pub fn to_string(x: Float) -> String {
-  x
-  |> string_builder.from_float
-  |> string_builder.to_string
+  do_to_string(x)
+}
+
+if erlang {
+  external fn do_to_string(Float) -> String =
+    "gleam_stdlib" "float_to_string"
+}
+
+if javascript {
+  external fn do_to_string(Float) -> String =
+    "../gleam_stdlib.mjs" "float_to_string"
 }
 
 /// Restricts a `Float` between a lower and upper bound.
diff --git a/src/gleam/string_builder.gleam b/src/gleam/string_builder.gleam
index 5e0054c..f1dfb30 100644
--- a/src/gleam/string_builder.gleam
+++ b/src/gleam/string_builder.gleam
@@ -14,7 +14,7 @@
 ///
 pub external type StringBuilder
 
-/// Create an empty `StringBuilder`. Useful as the start of a pipe chaning many
+/// Create an empty `StringBuilder`. Useful as the start of a pipe chaining many
 /// builders together.
 ///
 pub fn new() -> StringBuilder {
@@ -161,22 +161,6 @@ if javascript {
     "../gleam_stdlib.mjs" "length"
 }
 
-/// Creates a builder containing the textual representation of a given float.
-///
-pub fn from_float(f: Float) -> StringBuilder {
-  do_from_float(f)
-}
-
-if erlang {
-  external fn do_from_float(Float) -> StringBuilder =
-    "io_lib_format" "fwrite_g"
-}
-
-if javascript {
-  external fn do_from_float(Float) -> StringBuilder =
-    "../gleam_stdlib.mjs" "float_to_string"
-}
-
 /// Converts a builder to a new builder where the contents have been
 /// lowercased.
 ///
@@ -223,8 +207,18 @@ if erlang {
 }
 
 if javascript {
-  external fn do_reverse(StringBuilder) -> StringBuilder =
-    "../gleam_stdlib.mjs" "string_reverse"
+  import gleam/list
+
+  fn do_reverse(builder: StringBuilder) -> StringBuilder {
+    builder
+    |> to_string
+    |> do_to_graphemes
+    |> list.reverse
+    |> from_strings
+  }
+
+  external fn do_to_graphemes(string: String) -> List(String) =
+    "../gleam_stdlib.mjs" "graphemes"
 }
 
 /// Splits a builder on a given pattern into a list of builders.
@@ -303,7 +297,6 @@ if javascript {
 /// True
 /// ```
 ///
-///
 pub fn is_equal(a: StringBuilder, b: StringBuilder) -> Bool {
   do_is_equal(a, b)
 }
@@ -333,7 +326,6 @@ if javascript {
 /// True
 /// ```
 ///
-///
 pub fn is_empty(builder: StringBuilder) -> Bool {
   do_is_empty(builder)
 }
diff --git a/src/gleam_stdlib.erl b/src/gleam_stdlib.erl
index 6fd44ed..320d80b 100644
--- a/src/gleam_stdlib.erl
+++ b/src/gleam_stdlib.erl
@@ -10,7 +10,7 @@
          percent_encode/1, percent_decode/1, regex_check/2, regex_split/2,
          base_decode64/1, parse_query/1, bit_string_concat/1, size_of_tuple/1,
          decode_tuple/1, tuple_get/2, classify_dynamic/1, print/1, println/1,
-         inspect/1]).
+         inspect/1, float_to_string/1]).
 
 %% Taken from OTP's uri_string module
 -define(DEC2HEX(X),
@@ -374,4 +374,7 @@ inspect(Any) when is_function(Any) ->
     ),
     ["//fn(", Args, ") { ... }"];
 inspect(Any) ->
-    ["//erl(", io_lib:format("~p", [Any]), ")"].
\ No newline at end of file
+    ["//erl(", io_lib:format("~p", [Any]), ")"].
+
+float_to_string(Float) when is_float(Float) ->
+    erlang:iolist_to_binary(io_lib_format:fwrite_g(Float)).
diff --git a/src/gleam_stdlib.mjs b/src/gleam_stdlib.mjs
index b4f3d22..35c31c4 100644
--- a/src/gleam_stdlib.mjs
+++ b/src/gleam_stdlib.mjs
@@ -93,6 +93,12 @@ export function string_length(string) {
   }
 }
 
+export function graphemes(string) {
+  return List.fromArray(
+    Array.from(graphemes_iterator(string)).map(((item) => item.segment ))
+  );
+}
+
 function graphemes_iterator(string) {
   if (Intl && Intl.Segmenter) {
     return new Intl.Segmenter().segment(string)[Symbol.iterator]();
diff --git a/test/gleam/string_builder_test.gleam b/test/gleam/string_builder_test.gleam
index 14735ad..4c0d746 100644
--- a/test/gleam/string_builder_test.gleam
+++ b/test/gleam/string_builder_test.gleam
@@ -34,6 +34,34 @@ pub fn string_builder_test() {
   |> should.equal(13)
 }
 
+pub fn reverse_test() {
+  "Ĺo͂řȩm̅"
+  |> string_builder.from_string
+  |> string_builder.reverse
+  |> string_builder.reverse
+  |> string_builder.to_string
+  |> should.equal("Ĺo͂řȩm̅")
+
+  "Ĺo͂řȩm̅"
+  |> string_builder.from_string
+  |> string_builder.reverse
+  |> string_builder.to_string
+  |> should.equal("m̅ȩřo͂Ĺ")
+
+  "👶🏿"
+  |> string_builder.from_string
+  |> string_builder.reverse
+  |> string_builder.reverse
+  |> string_builder.to_string
+  |> should.equal("👶🏿")
+
+  "👶🏿"
+  |> string_builder.from_string
+  |> string_builder.reverse
+  |> string_builder.to_string
+  |> should.equal("👶🏿")
+}
+
 pub fn lowercase_test() {
   ["Gleam", "Gleam"]
   |> string_builder.from_strings