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

chore: cleanup, document, and test some RTL code #5473

Merged
merged 14 commits into from
Jul 27, 2024

Conversation

Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Jun 18, 2024

This PR is an attempt at cleaning up the RTL code and documentation a bit so it's easier to understand (that's subjective, I know).

  • Renamed containsRTL to lineContainsRTL_ and checked for lineContainsRTL_ || isRTL() in breakLine.
    This way, a few RTL words in a wrapped LTR message won't affect the lines below.
  • Renamed first and FirstWord to textDirection_ and Direction respectively.
    That's what they are about. first also "leaks" how it's detected (finding the first word that has a direction).
  • Added isRTL, isLTR, and isNeutral to check the text direction.
  • Moved from int to size_t and qsizetype (for signed values).
  • Added some assertions in addElement to check for weird usages of elements.
  • Moved from std containers (vector/stack) to QVarLengthArray.
    Single lines usually have very few elements, so we can store them on the stack (if that's not enough, the container will allocate).
  • Moved wasPrevReversed_ from a member to a local variable (isReversing)
  • Updated documentation for addElement and reorderRTL (and the other stuff I added)
  • Added basic tests for reordering.
    The tests only test the reordering on a single line. The line must have a username element. They're still a bit WIP, as e.g. images and mentions aren't tested.

@@ -60,7 +60,7 @@ Frames::Frames(QList<Frame> &&frames)
return init + frame.duration;
});

if (totalLength == 0)
if (totalLength <= 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not worth another PR imo.

@Nerixyz Nerixyz marked this pull request as ready for review July 24, 2024 12:29
@pajlada pajlada enabled auto-merge (squash) July 27, 2024 10:52
@pajlada pajlada merged commit ff7cc09 into Chatterino:master Jul 27, 2024
17 checks passed
@Nerixyz Nerixyz deleted the chore/rtl branch July 27, 2024 16:21
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.

2 participants