Re: [pgAdmin4][PATCH] Refactor and change of implementation of keyboard_shortcuts function dependencies

From: Wenlin Zhang <wzhang(at)pivotal(dot)io>
To: Sarah McAlear <smcalear(at)pivotal(dot)io>
Cc: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Violet Cheng <vcheng(at)pivotal(dot)io>
Subject: Re: [pgAdmin4][PATCH] Refactor and change of implementation of keyboard_shortcuts function dependencies
Date: 2017-08-10 06:31:28
Message-ID: CAEawo3KjD5pfc6YeATy-m1_pme7W75zA4GnPzeypxhMbkp1_eg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi hackers,

We have rebased all the three patches and they are ready to be applied.

Thanks,

Wenlin & Matt

On Wed, Aug 9, 2017 at 11:20 AM, Sarah McAlear <smcalear(at)pivotal(dot)io> wrote:

> Oh, great! Thanks Murtuza!
>
> On Tue, Aug 8, 2017 at 6:16 PM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> Hi Wenlin,
>>
>> Patch looks good to me.
>>
>> I'm also attaching minor patch to update document for new keyboard
>> shortcuts.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> [image: https://community.postgresrocks.net/]
>> <https://community.postgresrocks.net/>
>>
>> On Tue, Aug 8, 2017 at 1:41 PM, Wenlin Zhang <wzhang(at)pivotal(dot)io> wrote:
>>
>>> Hi Murtuza,
>>>
>>> Thanks for your review.
>>>
>>> We have fixed download bug, you can try it with the new patch
>>> 1.3_refactor_keyactions.diff, to see if it works.
>>> And we also changed the typo in the patch
>>> 2.1_change_keyboard_shortcuts_for_commenting_and_fix_typo.diff, ('client_plaform'
>>> -> 'client_platform').
>>>
>>> Thanks.
>>>
>>> Wenlin & Violet
>>>
>>> On Mon, Aug 7, 2017 at 2:30 PM, Murtuza Zabuawala <
>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Sarah,
>>>>
>>>> - Download button is not working, getting error on console (attaching
>>>> screenshot).
>>>> - While testing the patch I also observed another minor issue, If we
>>>> click on Explain & Explain analyze buttons, previous messages from Message
>>>> tab are not getting clear but instead new messages are getting appended to
>>>> previous ones.
>>>>
>>>> And could you please correct a typo introduced by Me from file
>>>> ../templates/datagrid/index.html, 'client_plaform' ->
>>>> 'client_platform' in your next patch as you working on it :)
>>>>
>>>> Rest of the changes looks good to me.
>>>>
>>>> --
>>>> Regards,
>>>> Murtuza Zabuawala
>>>> EnterpriseDB: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>>
>>>> On Mon, Aug 7, 2017 at 8:09 AM, Sarah McAlear <smcalear(at)pivotal(dot)io>
>>>> wrote:
>>>>
>>>>> Hi Murtuza,
>>>>>
>>>>> We have rebased and send the new patch.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Wenlin and Sarah
>>>>>
>>>>> On Fri, Aug 4, 2017 at 8:54 PM, Murtuza Zabuawala <
>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Sarah,
>>>>>>
>>>>>> Could you please rebase and send the patch again, I am not able to
>>>>>> apply any of the patch.
>>>>>>
>>>>>> murtuza(at)laptop:~/Documents/projects/pgadmin4$ git apply
>>>>>> ~/Downloads/1_refactor_keyactions.diff
>>>>>> error: patch failed: web/pgadmin/tools/sqleditor/st
>>>>>> atic/js/sqleditor.js:552
>>>>>> error: web/pgadmin/tools/sqleditor/static/js/sqleditor.js: patch
>>>>>> does not apply
>>>>>> murtuza(at)laptop:~/Documents/projects/pgadmin4$ git apply
>>>>>> ~/Downloads/2_change_keyboard_shortcuts_for_commenting.diff
>>>>>> error: patch failed: web/pgadmin/static/js/sqledito
>>>>>> r/keyboard_shortcuts.js:26
>>>>>> error: web/pgadmin/static/js/sqleditor/keyboard_shortcuts.js: patch
>>>>>> does not apply
>>>>>> error: patch failed: web/regression/javascript/sqle
>>>>>> ditor/keyboard_shortcuts_spec.js:181
>>>>>> error: web/regression/javascript/sqleditor/keyboard_shortcuts_spec.js:
>>>>>> patch does not apply
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Regards,
>>>>>> Murtuza Zabuawala
>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>> [image: https://community.postgresrocks.net/]
>>>>>> <https://community.postgresrocks.net/>
>>>>>>
>>>>>> On Fri, Aug 4, 2017 at 5:37 PM, Ashesh Vashi <
>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Murtuza,
>>>>>>>
>>>>>>> On Thu, Aug 3, 2017 at 3:38 PM, Sarah McAlear <smcalear(at)pivotal(dot)io>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi Hackers!
>>>>>>>>
>>>>>>>> Attached is a patch that extracts the functions called by the
>>>>>>>> keyboard_shortcuts function extracted earlier from sqlEditor. This
>>>>>>>> includes
>>>>>>>>
>>>>>>>> - executeQuery
>>>>>>>> - explainAnalyze
>>>>>>>> - explain
>>>>>>>> - download
>>>>>>>> - commentBlockCode
>>>>>>>> - commentLineCode
>>>>>>>> - uncommentLineCode
>>>>>>>>
>>>>>>>>
>>>>>>>> There is still more work to be done, but this is it for now.
>>>>>>>>
>>>>>>>> There is also an additional patch that changes the implementation
>>>>>>>> of the commentLineCode, uncommentLineCode, and commentBlockCode functions.
>>>>>>>> The shortcut
>>>>>>>>
>>>>>>>> - for commentLineCode is now cmd + / (ctrl + / for Windows)
>>>>>>>> - for uncommentLineCode cmd + . (ctrl + . for Windows)
>>>>>>>> - for comment and uncomment blockCode shift + cmd + / (shift +
>>>>>>>> ctrl + / for Windows)
>>>>>>>>
>>>>>>>> This is consistent with other IDEs and the way commenting is
>>>>>>>> implemented.
>>>>>>>>
>>>>>>> Please review it, and let us know your comments.
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>>> Thanks & Regards,
>>>>>>>
>>>>>>> Ashesh Vashi
>>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>>> <http://www.enterprisedb.com/>
>>>>>>>
>>>>>>>
>>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>> Hao & Sarah
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Attachment Content-Type Size
1.4_refactor_keyactions.diff text/plain 302.8 KB
2.1_change_keyboard_shortcuts_for_commenting_and_fix_typo.diff text/plain 9.3 KB
shortcuts_document_update.diff text/plain 1.5 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Surinder Kumar 2017-08-10 06:32:40 Re: [pgAdmin4][patch] update the alert style in the sub-navigation
Previous Message Murtuza Zabuawala 2017-08-10 06:25:04 Re: Can someone tell me what this code does ?