Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jot tests fail on JavaScript #3195

Closed
lpil opened this issue May 25, 2024 · 5 comments · Fixed by gleam-lang/stdlib#608
Closed

Jot tests fail on JavaScript #3195

lpil opened this issue May 25, 2024 · 5 comments · Fixed by gleam-lang/stdlib#608
Labels
bug Something isn't working help wanted Contributions encouraged priority:medium

Comments

@lpil
Copy link
Member

lpil commented May 25, 2024

I have not investigated yet but seeing as it's pure Gleam it means there's some bug in the compiler or the standard library causing it here.

https://github.com/lpil/jot

@lpil lpil added bug Something isn't working help wanted Contributions encouraged priority:medium labels May 25, 2024
@BradLewis
Copy link
Contributor

BradLewis commented May 26, 2024

So I had a look into this, and it looks like this is due to the difference in behaviour between the string.split_right implementation between erlang and javascript. The test(s) in question use unicode spaces (U+00A0), and the erlang implementation will keep the space, whereas the javascript version will strip it.

The relevant code is in the following function:

// jot.gleam
fn inlines_to_html(html: String, inlines: List(Inline), refs: Refs) -> String {
  case inlines {
    [] -> html
    [inline, ..rest] -> {
      html
      |> inline_to_html(inline, refs)
      |> inlines_to_html(rest, refs)
      |> string.trim_right
    }
  }
}

If we look at the bad test, with javascript we go from

"<p><em> a "
to
"<p><em> a"

whereas in erlang we keep the space.

I'm not sure what the desired behaviour is here though in order to fix it.

Erlang REPL:

❯ erl
Erlang/OTP 26 [erts-14.2.5] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit] [dtrace]

Eshell V14.2.5 (press Ctrl+G to abort, type help(). for help)
1> string:trim("<p><em> a ", trailing).
"<p><em> a "

Node REPL:

❯ node
Welcome to Node.js v18.12.1.
Type ".help" for more information.
> "<p><em> a ".trimRight()
'<p><em> a'

@lpil
Copy link
Member Author

lpil commented May 29, 2024

Good detective work! That is a rather annoying difference. 🤔

@BradLewis
Copy link
Contributor

Yeah definitely not ideal. I'm not sure how/if we would want to resolve this difference. Is there precedence elsewhere in the standard library where there is/was different behaviour between JS and Erlang?

@lpil
Copy link
Member Author

lpil commented Jun 2, 2024

We want them to behave the same here.

@BradLewis
Copy link
Contributor

In that case I have created a PR to rework the javascript behaviour to match the erlang behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Contributions encouraged priority:medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants