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

From: Sarah McAlear <smcalear(at)pivotal(dot)io>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: Wenlin Zhang <wzhang(at)pivotal(dot)io>, 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-09 03:20:21
Message-ID: CAGRPzo-2pQCsLGXOfQ1oJfnoC3zhEHSX_ZXn4qNtObW1=TMu9g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Oh, great! Thanks Murtuza!

On Tue, Aug 8, 2017 at 6:16 PM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)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
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Wenlin Zhang 2017-08-09 03:25:28 Re: [pgAdmin4[Patch][Webpacking]: Enable source maps in pgAdmin4 for development environment
Previous Message Shirley Wang 2017-08-08 14:17:40 Re: [pgAdmin4][Patch]: Allow user to cancel long running queries from dashboard