Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Nov 21, 2024

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

@pull pull bot added ⤵️ pull merge-conflict Resolve conflicts manually labels Nov 21, 2024
Earlopain and others added 14 commits December 25, 2024 18:22
When trying to calculate the width of such strings, it would previously
crash with either `Encoding::InvalidByteSequenceError` or
`Encoding::UndefinedConversionError`. Totally invalid characters are now simply
replaced with a replacement character when converting to UTF8.

Especially binary encoded strings (i.e. no encoding) don't make much sense but at least it doesn't
crash now and tries to return a sensible default (assume the string is actually valid UTF8
- Fix that modifiers were ignored when not part of a larger sequence #29
- Only check for valid base characters when in Emoji level is RGI
- Improve docs and specs
It's a simple function and I believe the result is not supposed to change at runtime.
Strings were not frozen, so it made a bunch of useless allocations too.

I found this method while benchmarking, which was unexpected.

Some basic numbers:
```
# frozen_string_literal: true

require 'unicode/display_width'
require 'benchmark/ips'

Benchmark.ips do |x|
  x.report do
    Unicode::DisplayWidth.of("foo")
  end

  x.compare!
end
```

Old gives `207.779k` iterations per second, while new one is `427.344k`.
So more than 2x faster for basic cases
Memoize `EmojiSupport.recommended`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⤵️ pull merge-conflict Resolve conflicts manually

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants