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

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.zabuawala@
> enterprisedb.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
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>