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

justenoughicu incorrect length calculation #839

Closed
mhosken opened this issue Mar 27, 2020 · 1 comment · Fixed by #840
Closed

justenoughicu incorrect length calculation #839

mhosken opened this issue Mar 27, 2020 · 1 comment · Fixed by #840
Assignees
Labels
bug Software bug issue
Milestone

Comments

@mhosken
Copy link
Contributor

mhosken commented Mar 27, 2020

In the function icu_bidi_runs there is a calculation to reduce the length of a run based on 2-part UTF-16 codes. There is a bug in this calculation. The loop walks forward reducing the length each time a trailing character is found. But this means that if there is more than one trailing character in a string, then the final character(s) will not be tested and the length may come out too long.

I suggest running the loop backwards.

@alerque alerque added the bug Software bug issue label Mar 27, 2020
@simoncozens simoncozens self-assigned this Mar 27, 2020
@simoncozens
Copy link
Member

Confirmed. Here's a spec test. Length is currently coming out as 4:

SILE = require("core/sile")
local icu = require("justenoughicu")

describe("SILE.linebreak", function()
  local chars = { 0x10000, 0x10001, 0x10002 }
  local utf8string = ""
  for i = 1,#chars do
    utf8string = utf8string .. SU.utf8char(chars[i])
  end

  it("should be the right length in UTF8", function()
    assert.is.equal(#utf8string, 12)
  end)

  it("should be the right length from ICU", function()
    local res = icu.bidi_runs(utf8string, "LTR")
    assert.is.equal(res.length, 3)
  end)
end)

simoncozens added a commit that referenced this issue Mar 27, 2020
Fixes #839. We were modifying a loop variable while in the loop, meaning
that we didn't run right to the end of the string.
simoncozens added a commit that referenced this issue Mar 27, 2020
Fixes #839. We were modifying a loop variable while in the loop, meaning
that we didn't run right to the end of the string.
@alerque alerque added this to the v0.10.4 milestone Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Software bug issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants