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

DolphinQt: Enable RTL layout #8006

Merged
merged 3 commits into from Oct 19, 2020
Merged

DolphinQt: Enable RTL layout #8006

merged 3 commits into from Oct 19, 2020

Conversation

JosJuice
Copy link
Member

QStringLiteral("Syrj"), QStringLiteral("Syrn"),
};

for (int i = 1; i < components.size(); ++i)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think theres a reason why this cannot be a range-based for-loop, can it?
Also, I wouldn't be surprised if there was an algorithm for this (altho I can't recall one from the top of my head)

Copy link
Member Author

Choose a reason for hiding this comment

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

It starts at 1 instead of 0 because I wanted to skip the main language code, so making it into a range-based for loop would be awkward.

Copy link
Member Author

Choose a reason for hiding this comment

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

The main language code should never actually be 4 in size, though...

Copy link
Member

Choose a reason for hiding this comment

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

Oops, kinda missed that :D

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of making this so hard on yourself you could just use
QStringList::takeFirst() to take away the first component which you can then use in line 205.

Now you would be able to use a for-range loop or better yet just turn your QStringList into a QSet<QString> and use
QSet::intersects (which will return true if at least one item matches the other)

Copy link
Member Author

Choose a reason for hiding this comment

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

I made it use QStringList::takeFirst and a range-based for loop now.

However, I can't just use QSet::intersects, because I want to return either true or false as soon as I find a script code (four-letter component). QSet::intersects only lets me find matching script codes, not non-matching script codes.

@leoetlino
Copy link
Member

Hmm, any thoughts on using the QT_LAYOUT_DIRECTION trick (where translators translate QT_LAYOUT_DIRECTION as either LTR or RTL themselves)? That'd remove the need for a hardcoded list of locales and scripts for which RTL is enabled.

@JosJuice
Copy link
Member Author

JosJuice commented May 4, 2019

Does that work for Qt Widgets? I only found it in the documentation for Qt Quick.

@leoetlino
Copy link
Member

@JosJuice
Copy link
Member Author

JosJuice commented May 5, 2019

That's probably the way to go, then. I'll look into it.

@leoetlino
Copy link
Member

Any updates on this?

@JosJuice
Copy link
Member Author

No, I've been busy with other things. But I'm still planning to get around to it at some point. The reason this isn't a trivial change is because I believe I have to add some missing features to our translations loading code.

@leoetlino leoetlino marked this pull request as draft May 3, 2020 16:36
@JosJuice
Copy link
Member Author

JosJuice commented Jun 2, 2020

Done.

As part of making this change, I had to update msgfmt.exe in Externals. However, I can't find where we keep the source code for this executable. We do need to have the source code of it somewhere in order to comply with the GPL, right...?

@JosJuice JosJuice marked this pull request as ready for review June 2, 2020 20:29
@BhaaLseN
Copy link
Member

BhaaLseN commented Jun 3, 2020

msgfmt is part of Gettext; isn't it enough to point that out somewhere?

@lioncash lioncash merged commit 5722c68 into dolphin-emu:master Oct 19, 2020
@JosJuice JosJuice deleted the qt-rtl branch October 19, 2020 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants