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

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