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

On Wed, Aug 16, 2017 at 8:55 AM, Violet Cheng <vcheng(at)pivotal(dot)io> wrote:

> Hi,
>
> Is there any update on these patches? Will it be committed soon?
>
Dave - would please take care of this?

--

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,
> Violet
>
> On Thu, Aug 10, 2017 at 2:31 PM, Wenlin Zhang <wzhang(at)pivotal(dot)io> wrote:
>
>> 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(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
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>