Re: [pgAdmin4][PATCH] Refactor and change of implementation ofkeyboard_shortcuts function dependencies - Mailing list pgadmin-hackers

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@pivotal.io> wrote:

Oh, great! Thanks Murtuza!

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

On Tue, Aug 8, 2017 at 1:41 PM, Wenlin Zhang <wzhang@pivotal.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@pivotal.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@laptop:~/Documents/projects/pgadmin4$ git apply ~/Downloads/1_refactor_keyactions.diff

error: patch failed: web/pgadmin/tools/sqleditor/static/js/sqleditor.js:552

error: web/pgadmin/tools/sqleditor/static/js/sqleditor.js: patch does not apply

murtuza@laptop:~/Documents/projects/pgadmin4$ git apply ~/Downloads/2_change_keyboard_shortcuts_for_commenting.diff

error: patch failed: web/pgadmin/static/js/sqleditor/keyboard_shortcuts.js:26

error: web/pgadmin/static/js/sqleditor/keyboard_shortcuts.js: patch does not apply

error: patch failed: web/regression/javascript/sqleditor/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

On Fri, Aug 4, 2017 at 5:37 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Murtuza,

On Thu, Aug 3, 2017 at 3:38 PM, Sarah McAlear <smcalear@pivotal.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.linkedin.com/in/asheshvashi 

Thanks!
Hao & Sarah