TwilioDevEd / message-segment-calculator

JS-based tool to calculate and display message segmentation data for a given message body
https://twiliodeved.github.io/message-segment-calculator/
MIT License
85 stars 35 forks source link

Incorrect segment counting #18

Closed MikeCarbone closed 2 years ago

MikeCarbone commented 2 years ago

Hello! Just wanted to let you know that the segment counting doesn't seem to be done correctly. I was building my own implementation of a segment counter and stumbled upon an innaccuracy.

Take this ridiculous string for example: this is a ]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]😘

The tool says this is 3 segments.

However, my understanding is that it should be 2 segments. The length of the string is 86, with the emoji triggering UCS-2 encoding. However once switched to UCS-2, the special GSM-7 bracket should now only count as one character with a maximum per segment of 67 characters.

I believe the tool may be double counting the special GSM-7 counting, then not adjusting once UCS-2 is triggered.

I sent that string to Twilio and it does indeed count as two segments, not three.

Anyways, not terribly urgent, just figured I'd let you all know. Thanks for the tool! Really helped me wrap my head around the topic.

vernig commented 2 years ago

Hey @MikeCarbone, you are absolutely right! Thanks for raising this issue. Let me work on a fix.

MikeCarbone commented 2 years ago

Nice! Thanks @vernig! I created the same problem in my implementation originally. I ended up fixing it by keeping track of normal GSM-7 characters and special GSM-7 characters in two separate counts, then doing the math after running through the string. If it was GSM-7, I'd multiply the special-GSM count * 2, then add it with the normal count. If it was UCS-2 at the end, I'd just add the counts together for a total.

You probably don't need that, but figured I'd share. Thanks for the quick response! 👍

vernig commented 2 years ago

I can see what you mean @MikeCarbone and thanks for sharing your implementation. I may adopt a very similar solution. Let me do some more test and get back to you

vernig commented 2 years ago

:tada: This issue has been resolved in version 1.1.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: