Skip to content
New issue

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

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

Already on GitHub? # to your account

Clean up the code of RichTextAnnotatedStringParser #39

Merged
merged 2 commits into from
May 22, 2023
Merged

Clean up the code of RichTextAnnotatedStringParser #39

merged 2 commits into from
May 22, 2023

Conversation

AzimMuradov
Copy link
Contributor

Foremost, thanks for the great library, we really needed that in compose multiplatform.

About PR:

Before release of 0.2.0 I tried to implement similar encoding and decoding functionality. I think the suggested solution is better.

For encode, only the functional approach is used (compare to the mix of approaches previously).

I also used destruction declaration for the SpanStyle and RichTextPart.

@MohamedRejeb
Copy link
Owner

MohamedRejeb commented May 22, 2023

Thanks for your contribution, I really liked the destruction it makes it cleaner.
For the currentStyles the previous approach is better in term of performance, we only need to loop the parts a single time.

@MohamedRejeb MohamedRejeb self-requested a review May 22, 2023 10:49
}

val currentStyles = parts
Copy link
Owner

@MohamedRejeb MohamedRejeb May 22, 2023

Choose a reason for hiding this comment

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

The previous approach is better in term of performance, we only need to loop the parts a single time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, reverted some of the changes.

@MohamedRejeb MohamedRejeb self-requested a review May 22, 2023 10:54
Copy link
Owner

@MohamedRejeb MohamedRejeb left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution

@MohamedRejeb MohamedRejeb merged commit 0cc7f65 into MohamedRejeb:main May 22, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants