Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure

From: Rahul Shirsat <rahul(dot)shirsat(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure
Date: 2021-06-29 15:50:10
Message-ID: CAKtn9dNWNAO84Qg15TPoJgVMgah39jbQcxaWp6xDik2ykQQx-g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave / Aditya,

For a time being, Let's make a call to gettext conditional instead of
passing dynamic parameters for this scenario at least. With this, we won't
be touching the *.po files and translations will do its task smoothly.
I have already checked for the string with weird unprintable characters,
and there were none.

e.g.
Instead of - *title = gettext('%s objects?', obj.label);*

*if (drop) {*
* title = gettext('Drop objects?');*
*}*
*else {*
*title = gettext('Reassign objects?');*
*}*

I have tried the above code and it looks good to me.

Please suggest.

On Tue, Jun 29, 2021 at 7:31 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> On Tue, Jun 29, 2021 at 2:55 PM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> On Tue, Jun 29, 2021 at 7:14 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Tue, Jun 29, 2021 at 2:41 PM Aditya Toshniwal <
>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>
>>>> Dave,
>>>>
>>>> Somehow, the new text strings are added to PO with incorrect
>>>> translations. That is causing the issue.
>>>> Either they should be empty or fixed.
>>>>
>>>
>>> Then the source problem should be fixed. There's no point at all in
>>> putting fixes directly in the PO files as they'll be overwritten prior to
>>> release anyway.
>>>
>> The translations submitted by translators are updated in the PO file
>> right ? And then they're compiled to MO ?
>>
>
> Right.
>
>
>> It's the same like Rahul will be submitting the translations. Please
>> correct me if I'm wrong.
>>
>
> That depends if he's changed the original message or the translated
> message. It's impossible to see without reading megabytes of text.
>
> And in any case, he's updated translations that haven't been touched in
> ages - why would they suddenly have broken? (hint: if the upstream message
> has changed, it'll be marked as fuzzy or untranslated - in other words,
> gettext knows how to handle that).
>
>
>>
>>>
>>>>
>>>> On Tue, Jun 29, 2021 at 7:01 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> Please send the patch without updates to the po files. Those get
>>>>> updated as part of the release process.
>>>>>
>>>>> Thanks.
>>>>>
>>>>> On Tue, Jun 29, 2021 at 2:00 PM Rahul Shirsat <
>>>>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Hackers,
>>>>>>
>>>>>> Thanks Aditya for pointing out the issue. Please find the attached
>>>>>> patch which contains all the .po files corrected with %s.
>>>>>>
>>>>>> Regards,
>>>>>> Rahul Shirsat.
>>>>>>
>>>>>> On Tue, Jun 29, 2021 at 4:31 PM Aditya Toshniwal <
>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Rahul,
>>>>>>>
>>>>>>> I did "make msg-extract" and "make msg-update" and looking at the
>>>>>>> PO files I think it is not updated correctly.
>>>>>>> For instance, the below message has msgstr without %s. I corrected
>>>>>>> it and the error was gone.
>>>>>>>
>>>>>>> #: pgadmin/browser/server_groups/servers/roles/static/js/role.js:766
>>>>>>> #, fuzzy, python-format
>>>>>>> msgid "%s Objects"
>>>>>>> msgstr "Obiekty"
>>>>>>>
>>>>>>> The one below had 2 %s in msgstr and I corrected it to fix the error.
>>>>>>>
>>>>>>> #: pgadmin/browser/server_groups/servers/roles/static/js/role.js:767
>>>>>>> #, fuzzy, python-format
>>>>>>> msgid "Are you sure you wish to %s all the objects owned by the
>>>>>>> selected role?"
>>>>>>> msgstr "Czy na pewno skasować %s \"%s\" i wszystkie obiekty zależne
>>>>>>> od niego?"
>>>>>>>
>>>>>>>
>>>>>>> You have to update the .po files to match the total %s and send the
>>>>>>> patch.
>>>>>>>
>>>>>>> On Tue, Jun 29, 2021 at 1:56 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Tue, Jun 29, 2021 at 4:38 AM Rahul Shirsat <
>>>>>>>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> I feel gettext sometimes won't escape the characters as it should
>>>>>>>>> be.
>>>>>>>>>
>>>>>>>>> I now tried to escape those using some utils.
>>>>>>>>>
>>>>>>>>
>>>>>>>> That won't work either. The string being passed to gettext() *must*
>>>>>>>> be in the gettext call.
>>>>>>>>
>>>>>>>> When gettext extracts strings to create/update the catalogs, it
>>>>>>>> will search the code for all gettext calls, and then extract a string
>>>>>>>> constant from the first argument. You cannot have variables, function calls
>>>>>>>> or expressions in there. It *must* be a string constant.
>>>>>>>>
>>>>>>>> Keep in mind that msgextract is scanning the source code; it's not
>>>>>>>> executing it. There are many examples in the code, e.g. (from node.js):
>>>>>>>>
>>>>>>>> title = gettext('Drop %s?', obj.label);
>>>>>>>>
>>>>>>>> I don't see anything obviously wrong with the existing code. Are
>>>>>>>> you sure there are no weird unprintable characters in there?
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Please find the updated patch.
>>>>>>>>>
>>>>>>>>> On Mon, Jun 28, 2021 at 9:33 PM Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Hi
>>>>>>>>>>
>>>>>>>>>> On Mon, Jun 28, 2021 at 4:57 PM Rahul Shirsat <
>>>>>>>>>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>
>>>>>>>>>>> Please find the attached patch for fixation of jenkins failure.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> That won't work - you can't include variables (or string building
>>>>>>>>>> operations) in the first argument to gettext calls, as there won't be any
>>>>>>>>>> way to extract a complete message into the catalogs. The way it's being
>>>>>>>>>> done at the moment is correct (I don't know why it's failing, but it's the
>>>>>>>>>> correct way to structure the gettext calls).
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Dave Page
>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>
>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> *Rahul Shirsat*
>>>>>>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Dave Page
>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>> Twitter: @pgsnake
>>>>>>>>
>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Thanks,
>>>>>>> Aditya Toshniwal
>>>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>>>>> <http://edbpostgres.com>
>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> *Rahul Shirsat*
>>>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Dave Page
>>>>> Blog: https://pgsnake.blogspot.com
>>>>> Twitter: @pgsnake
>>>>>
>>>>> EDB: https://www.enterprisedb.com
>>>>>
>>>>>
>>>>
>>>> --
>>>> Thanks,
>>>> Aditya Toshniwal
>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>> <http://edbpostgres.com>
>>>> "Don't Complain about Heat, Plant a TREE"
>>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: https://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EDB: https://www.enterprisedb.com
>>>
>>>
>>
>> --
>> Thanks,
>> Aditya Toshniwal
>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>> <http://edbpostgres.com>
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> Dave Page
> Blog: https://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EDB: https://www.enterprisedb.com
>
>

--
*Rahul Shirsat*
Senior Software Engineer | EnterpriseDB Corporation.

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Rahul Shirsat 2021-06-30 07:28:11 Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure
Previous Message Dave Page 2021-06-29 14:00:55 Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure