In D14779#308593, @habacker wrote:

You have remembered that in this case the loader created by ecm_create_qm_loader(kcoreaddons_QM_LOADER kcoreaddons5_qt) must be adjusted to find this - not installed - fake translation by default ?

Personally i would include (and use) a fake translation as part of the tests and then it would just evaluate to the actual result, i.e.

QCOMPARE(format.formatSpelloutDuration(3610000), QStringLiteral("1 hour(s)"));

would end up being

QCOMPARE(format.formatSpelloutDuration(3610000), QStringLiteral("1 hour"));

because we're using the "fake" translation.

In D14820#308569, @sander wrote:

Looking good! Concerning the preparatory commit 9ba8dd2cd7838c626af79a4edcd8b8437205cc02 , which copies the list of loaded generators from a QHash to a QMap: Why not use a QMap for them right away? Certainly the number of generators is low enough such that the efficiency differences between a QHash and a QMap become negligible?

Honestly i think that test in kformattest is wrong and that it should contain it's own small minitranslation so that stuff works fine and then it should be

Looks good, to me, since it seems to me you're serious in being a KDE Developer, i think you should get a Developer account

I made https://phabricator.kde.org/D14820 (you need latest branch since i did a change to make the sorting at least stable in the config dialog) which doesn't need changing the API, which i think it's better since it won't break plugins that may exist outside the okular repository.

Maybe Ctrl+Shift instead of Ctrl+Alt?

This seems very corner casey and i'm afraid we may be adding too much menu entries, but ok.

Yes he can.

add small comment

Doesn't setting the locale to en solve the same problem without needing to change ECM?

Do you think you could work on an autotest for this?

i'm not very happy with m_remainingTeaTime as a variable name, because to me it doesn't say "this is only for when it's paused", maybe m_pausedRemainingTeaTime?

looks good, if you can please add const to all those variables.

Looks good to me, if you want, maybe it makes sense to put

Isn't this actually bad for the privacy goal? AFAIK some apps will just write the password in cleartext in a config file when kwallet is not present.

What to link https://www.kde.org/applications/development/lokalize/ ?

Looks good :)

As far as i can see none of these headers get installed so should they all be renamed to _p.h ?

Simon what do you think about this?

As I am arguing in the other review, let's install the files in a nicer place? We control were we install them, no?

Ok, second option, we are actually installing these files ourselves, right?

you still need to merge 5.13 to master because eventually someone will, it's the trouble with cherry-picking.

Is there a way we can get /../qt-reserved-files/share/ in a more natural way? fishing up to .. feels always like it'll break

Do i understand it correctly that we only remove the file on

Aleix means https://phabricator.kde.org/D14508

Maybe it makes more sense to replace it with the magic words?

I would feel more safe if you initialized m_cursorSelectionStart and m_cursorSelectionEnd to 0 in the constructor

This makes it a bit better so i will commit it, but it is still not as good as it could/should be. See https://i.imgur.com/HDn9QIX.png

Are we sure this won't break something else?

I'm not going to pretend that i had time to read and understand the code, but if you think it's better and you'll fix any bugs that it may introduce, just go ahead and commit it :)

In D14411#298809, @ckertesz wrote:

In D14411#298807, @aacid wrote:

In D14411#298786, @pino wrote:

This does not look good for different reasons:

• the KI18n framework is marked as working on Android (and looking at its git history, there were fixes for it); hence, if it still does not work, please fix it instead (so it will work for kstars any any other KI18n-based application)

It does work, i've used it.

Where? Do you have any code online? My Google search did not give any results. I asked Aleix Pol some days ago and he could not reference any KDE app which uses KI18n with gettext-style translations on Android.

In D14411#298786, @pino wrote:

This does not look good for different reasons:

• the KI18n framework is marked as working on Android (and looking at its git history, there were fixes for it); hence, if it still does not work, please fix it instead (so it will work for kstars any any other KI18n-based application)

Does this really make a difference?

In D14219#297396, @sdepiets wrote:

In D14219#297294, @aacid wrote:

Should the state of the action be remembered between runs?

I didn't plan that, the quick search project field isn't saved between runs.
Performance wise it would not change anything to launch the program with the filter on.

Should the state of the action be remembered between runs?

If it works, sure let's go for it.

I may be available on Sunday.

What is a "Deleted file"? A file that did exist and does not exist anymore?

I think this is fine, if we let people zoom on the top area, we should let people zoom on the zoom one, but i agree with Safa this is not the actual fix for bug 375251