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

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
>>>>>
>>>>
>>>>
>>>
>>
>