Merge lp://qastaging/~nikwen/ubuntu-calendar-app/fix-standalone-month-name-i18n into lp://qastaging/ubuntu-calendar-app

PASSED: Continuous integration, rev:735
https://core-apps-jenkins.ubuntu.com/job/calendar-app-ci/23/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/221/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/calendar-app-ci/23/rebuild

PASSED: Continuous integration, rev:736
https://core-apps-jenkins.ubuntu.com/job/calendar-app-ci/24/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/222/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/calendar-app-ci/24/rebuild

Thanks :), this changes seems fine to me, but need to test is once and then approve it

Why do you need to call qsTr()? IIRC, it should work with i18n.tr() only already.

qsTr(i18n.tr("%1 %2")).arg(Qt.locale().standaloneMonthName(currentMonth.getMonth(), Locale.LongFormat)).arg(currentMonth.getFullYear())

Sorry, to be more specific:

- Qt.locale() should choose the right locale for you already for the standaloneMonthName, without needing qsTr()
- Does currentMonth.getFullYear() not need to be localized as well?

Thank you, Kunal and David, for looking into this.

@David, to clear this up a bit:

qsTr() is responsible only for the format string here. i18n.tr() delivers the translated format string while qsTr() in combination with the arg() calls then goes ahead and replaces %1 and %2 with the arguments, i.e. the month and the year. That said, qsTr() does not do any translation job and is only needed for the format string story. I couldn't find anything in the i18n documentation to replace the qsTr() and arg() calls here. [1]
Regarding the year: Qt.locale() does not provide any way to localize the year. [2] Hence, I don't think we need to do anything, especially considering that we are talking about a number.

[1] https://developer.ubuntu.com/api/apps/qml/sdk-15.04.1/Ubuntu.Components.i18n/
[2] http://doc.qt.io/qt-5/qml-qtqml-locale.html

Any update on this? Kunal, have you given it a try yet?

I've just double-checked this again, and I can confirm that the qsTr() call is not required. i18n.tr() (gettext) already takes care of the format string, it is not necessary to process it again with qsTr().

This can easily be tested with a simple QML app that includes the following string:

i18n.tr("%1 %2").arg(Qt.locale().standaloneMonthName(10, Locale.LongFormat)).arg("2011")

FAILED: Continuous integration, rev:737
https://core-apps-jenkins.ubuntu.com/job/calendar-app-ci/748/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/557/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/calendar-app-ci/748/rebuild

So I just learned something new. Thanks, David! :)

(By the way, would it be possible to add an example for that to https://developer.ubuntu.com/api/apps/qml/sdk-15.04.1/Ubuntu.Components.i18n/ ?)

Now I just have to sort out the merge conflicts.

Fixed the merge conflicts. Would you mind looking into it again, please? :)

FAILED: Continuous integration, rev:738
https://core-apps-jenkins.ubuntu.com/job/run-ap-tests-ci/1007/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/558/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/run-ap-tests-ci/1007/rebuild

Looking at the Jenkins logs, I don't think the test failures are related to the changes here. (Please correct me if I'm wrong, though!) Any idea what was happening?

No ideas?

@nikwen, Hi there, we're looking to release calendar v0.5 with OTA-10. I would like to get this bug fix into trunk before that. Can we work on getting this done?

I have two questions,

1. why at all do we need i18n.tr() calls? Can we just do "%1 %2".arg().arg() which then substitutes the month and year accordingly? Or is this for RTL language support where the order may need to be changed?

2. calendar app trunk has gone through a huge code change. Please merge trunk and push again pls.

Hey Nekhelesh,

I'm sorry for being a bit late.

The reason why I put the i18n.tr() calls in there is because there might be languages in which it is "2016 April" instead of "April 2016". I'm not sure about whether there are any such languages, but if we are already fixing translation issues, I'd prefer to make it as flexible as possible. Furthermore, in my opinion the i18n.tr() calls don't hurt anyone if no such languages exist.

I'll merge trunk now. :)

PASSED: Continuous integration, rev:742
https://core-apps-jenkins.ubuntu.com/job/calendar-app-ci/825/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/856/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/calendar-app-ci/825/rebuild

Finished merging. Thank you for your patience, Nekhelesh! :)

Thanks Niklas, and good work everyone!

On Wed, Apr 6, 2016 at 10:38 PM, Niklas Wenzel <email address hidden>
wrote:

> Finished merging. Thank you for your patience, Nekhelesh! :)
> --
>
> https://code.launchpad.net/~nikwen/ubuntu-calendar-app/fix-standalone-month-name-i18n/+merge/279509
> You are reviewing the proposed merge of
> lp:~nikwen/ubuntu-calendar-app/fix-standalone-month-name-i18n into
> lp:ubuntu-calendar-app.
>

Nekhelesh, would you mind having a look at this again? :)