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

fix: straddle message bug #6

Closed
wants to merge 1 commit into from
Closed

fix: straddle message bug #6

wants to merge 1 commit into from

Conversation

atymic
Copy link

@atymic atymic commented Jul 14, 2021

Closes #1

Maybe worth some further tests as well, once merged i'll check the trucate PR works as expected

@chrisminett
Copy link
Member

Thanks @atymic ! I'll have a look at this in the next week or so. I've been going through the Phlib repos to upgrade them to minimum PHP 7.3 so I'll do this one next, including adding the CI tests which don't appear to have run here.

@chrisminett chrisminett self-assigned this Jul 14, 2021
@atymic
Copy link
Author

atymic commented Jul 14, 2021

No worries, it did actually run because i got emails from travis haha.

@atymic
Copy link
Author

atymic commented Jul 14, 2021

Want me to PR bump to 7.3 and github actions CI?

['', '7-bit', 0, 1, 160]
['', '7-bit', 0, 1, 160],

[
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment explaining what the test is checking, please? Something which explains the value, like in #1 would be great:

The '[' at character 152 will become "0x1B[" (2 characters), and become part of the 2nd message, which pushes the last "." at the end of the content into a 3rd message

@chrisminett
Copy link
Member

@atymic I've done all the code style changes I wanted to get done. Could you please rebase your changes from main? I think the only differences would be type declarations and named unit test datasets.

Love the check for the modulo! 🚀

@atymic atymic closed this Jul 19, 2021
@atymic
Copy link
Author

atymic commented Jul 19, 2021

See #8

Someone your changes to the repo broke my local git to the point where I could not push at all! Had to delete fork and re-pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Miscalculates number of messages when extended character is split between messages
2 participants