Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-26 Thread Dave Page
Thanks, applied. I also extended the code you added to ensure the labels
for the Find/Replace options are now platform-correct.

On Wed, Jul 26, 2017 at 10:29 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> Please find updated patch for new shortcut keys, I have tested it on all
> three major platforms (macOS, Linux & Windows with Chrome, FF & IE11
> Browsers).
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Fri, Jul 21, 2017 at 9:38 PM, Robert Eckhardt 
> wrote:
>
>> Right - we lose the ability to uncomment multiple levels though, which
>>> may also be useful.
>>>
>>
>> Well yes.
>>
>> I would argue that simplicity trumps potential use. I'd also argue that
>> attempting to maintain consistency across environments (IDEs, etc. ) is
>> advantageous. This was the philosophy we were going with when enabling
>> excel like behavior (also why I'm not fully happy with how it is today).
>>
>> -- Rob
>>
>
>


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-26 Thread Murtuza Zabuawala
Hi Dave,

Please find updated patch for new shortcut keys, I have tested it on all
three major platforms (macOS, Linux & Windows with Chrome, FF & IE11
Browsers).

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Jul 21, 2017 at 9:38 PM, Robert Eckhardt 
wrote:

> Right - we lose the ability to uncomment multiple levels though, which may
>> also be useful.
>>
>
> Well yes.
>
> I would argue that simplicity trumps potential use. I'd also argue that
> attempting to maintain consistency across environments (IDEs, etc. ) is
> advantageous. This was the philosophy we were going with when enabling
> excel like behavior (also why I'm not fully happy with how it is today).
>
> -- Rob
>
diff --git a/docs/en_US/keyboard_shortcuts.rst 
b/docs/en_US/keyboard_shortcuts.rst
index cd0938d..32f06f0 100644
--- a/docs/en_US/keyboard_shortcuts.rst
+++ b/docs/en_US/keyboard_shortcuts.rst
@@ -41,11 +41,11 @@ When using the syntax-highlighting SQL editors, the 
following shortcuts are avai
 
+--+--+-+
 | Ctrl+Alt+Right   | Cmd+Option+Right | Move right one word
 |
 
+--+--+-+
-| Ctrl+Shift+, | Ctrl+Shift+, | Comment selected code (Inline) 
 |
+| Ctrl+Shift+/ | Cmd+Shift+/  | Comment selected code (Inline) 
 |
 
+--+--+-+
-| Ctrl+Shift+. | Ctrl+Shift+. | Uncomment selected code 
(Inline)|
+| Ctrl+Shift+. | Cmd+Shift+.  | Uncomment selected code 
(Inline)|
 
+--+--+-+
-| Ctrl+Shift+/ | Ctrl+Shift+/ | Comment/Uncomment code (Block) 
 |
+| Ctrl+/   | Cmd+/| Comment/Uncomment code (Block) 
 |
 
+--+--+-+
 | Ctrl+A   | Cmd+A| Select all 
 |
 
+--+--+-+
diff --git a/web/pgadmin/static/js/sqleditor/keyboard_shortcuts.js 
b/web/pgadmin/static/js/sqleditor/keyboard_shortcuts.js
index c117413..5d947d1 100644
--- a/web/pgadmin/static/js/sqleditor/keyboard_shortcuts.js
+++ b/web/pgadmin/static/js/sqleditor/keyboard_shortcuts.js
@@ -1,9 +1,9 @@
 const F5_KEY = 116,
   F7_KEY = 118,
   F8_KEY = 119,
-  COMMA_KEY = 188,
   PERIOD_KEY = 190,
-  FWD_SLASH_KEY = 191;
+  FWD_SLASH_KEY = 191,
+  IS_CMD_KEY = window.navigator.platform.search('Mac') != -1;
 
 function keyboardShortcuts(sqlEditorController, event) {
   if (sqlEditorController.isQueryRunning()) {
@@ -24,13 +24,16 @@ function keyboardShortcuts(sqlEditorController, event) {
   } else if (keyCode === F8_KEY) {
 event.preventDefault();
 sqlEditorController.download();
-  } else if (event.shiftKey && event.ctrlKey && keyCode === COMMA_KEY) {
+  } else if (((IS_CMD_KEY && event.metaKey) || (!IS_CMD_KEY && event.ctrlKey)) 
&&
+ event.shiftKey && keyCode === FWD_SLASH_KEY) {
 _stopEventPropagation();
 sqlEditorController.commentLineCode();
-  } else if (event.shiftKey && event.ctrlKey && keyCode === PERIOD_KEY) {
+  } else if (((IS_CMD_KEY && event.metaKey) || (!IS_CMD_KEY && event.ctrlKey)) 
&&
+ event.shiftKey && keyCode === PERIOD_KEY) {
 _stopEventPropagation();
 sqlEditorController.uncommentLineCode();
-  } else if (event.shiftKey && event.ctrlKey && keyCode === FWD_SLASH_KEY) {
+  } else if (((IS_CMD_KEY && event.metaKey) || (!IS_CMD_KEY && event.ctrlKey)) 
&&
+ keyCode === FWD_SLASH_KEY) {
 _stopEventPropagation();
 sqlEditorController.commentBlockCode();
   }
diff --git a/web/pgadmin/tools/datagrid/__init__.py 
b/web/pgadmin/tools/datagrid/__init__.py
index 08b01ab..d3a4a9d 100644
--- a/web/pgadmin/tools/datagrid/__init__.py
+++ b/web/pgadmin/tools/datagrid/__init__.py
@@ -15,6 +15,7 @@ import pickle
 import random
 
 from flask import Response, url_for, session, request, make_response
+from werkzeug.useragents import UserAgent
 from flask import current_app as app
 from flask_babel import gettext
 from flask_security import login_required
@@ -183,6 +184,9 @@ def panel(trans_id, is_query_tool, editor_title):
 else:
 sURL = None
 
+# We need client OS information to render correct Keyboard shortcuts
+user_agent = UserAgent(request.headers.get('User-Agent'))
+
 """
 Animations and transitions are not automatically GPU accelerated and by 
default use browser's slow rendering engine.
 We need to set 'translate3d' value of '-webkit-transform' property in 
order to use GPU.
@@ -212,7 +216,8 @@ def panel(trans_id, is_query_tool, editor_title):
 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-21 Thread Robert Eckhardt
I'd say we should go with Cmd+/ and Cmd+Shift+/ as that seems the most
common by a small margin - however, I'm still not convinced that we don't
need two keys for optimal behaviour of line commenting.


To be clear, I'm not 100% convinced either. I am, however, convinced that
it is the right first step and then we can get user feedback.

-- Rob


Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-21 Thread Dave Page
On Fri, Jul 21, 2017 at 8:57 PM, Shirley Wang  wrote:

> Until we learn definitively from users that the current implementation of
> commenting/uncommenting in other tools is not working, we should use what
> is common practice. We can use that as a baseline and then if we learn that
> there needs to be another shortcut, we can add additional functionality.
> It's much easier to add something for users than to take away.
>

There isn't really any common practice as far as I can see. In a 20 minute
search I found the following variations (substitute Ctrl for Cmd on
non-Mac):

Tools supporting only one comment mode:

XCode: Cmd+/
Visual Studio: Ctrl+K+C Ctrl+K+U
SQL Server Management Studio: Ctrl+K+C Ctrl+K+U
pgAdmin 3: Cmd+K/Cmd+Shift+K
Editra: Cmd+1
Netbeans: Cmd+Shift+C
Eclipse: Cmd+/
Sublime Text: Cmd+/ Cmd+Shift+/
Toad: Ctrl+b Ctrl+Shift+b
MySQL Workbench: Cmd+/

Tools supporting line and block comment modes:

PyCharms (line comment): Cmd+/
PyCharms (block comment): Cmd+Alt+/
Notepad++ (line comment): Ctrl+q
Notepad++ (block comment): Ctrl+Shift+q

I'd say we should go with Cmd+/ and Cmd+Shift+/ as that seems the most
common by a small margin - however, I'm still not convinced that we don't
need two keys for optimal behaviour of line commenting.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-21 Thread Shirley Wang
Until we learn definitively from users that the current implementation of
commenting/uncommenting in other tools is not working, we should use what
is common practice. We can use that as a baseline and then if we learn that
there needs to be another shortcut, we can add additional functionality.
It's much easier to add something for users than to take away.

On Fri, Jul 21, 2017 at 12:08 PM Robert Eckhardt 
wrote:

> Right - we lose the ability to uncomment multiple levels though, which may
>> also be useful.
>>
>
> Well yes.
>
> I would argue that simplicity trumps potential use. I'd also argue that
> attempting to maintain consistency across environments (IDEs, etc. ) is
> advantageous. This was the philosophy we were going with when enabling
> excel like behavior (also why I'm not fully happy with how it is today).
>
> -- Rob
>


Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-21 Thread Dave Page
On Fri, Jul 21, 2017 at 4:43 PM, Robert Eckhardt 
wrote:

> I understand now.
>
> If any one line is different then the entire thing will be commented so in
> your example:
>
> --CREATE TABLE foo (
>  -- id serial,
>  -- data text
> --);
>
> ---- Index required on data for finding Wumpus' quickly . (not sure if
> there should be spaces or not)
> --CREATE INDEX foo_idx ON foo (data);
>
> Similarly if we have all but one commented sterting like this:
>
>
> --CREATE TABLE foo (
>  -- id serial,
>  -- data text
> --);
>
> Index required on data for finding Wumpus' quickly . (not sure if there
> should be spaces or not)
> --CREATE INDEX foo_idx ON foo (data);
>
>
> It will then look like this (again spaces optional):
>
> --  --CREATE TABLE foo (
> --   -- id serial,
> --   -- data text
> --  --);
>
> --Index required on data for finding Wumpus' quickly . (not sure if there
> should be spaces or not)
> --  --CREATE INDEX foo_idx ON foo (data);
>
>
Right - we lose the ability to uncomment multiple levels though, which may
also be useful.


>
>
> -- Rob
>
> On Fri, Jul 21, 2017 at 11:29 AM, Dave Page  wrote:
>
>>
>>
>> On Fri, Jul 21, 2017 at 4:21 PM, Robert Eckhardt 
>> wrote:
>>
>>> I hope you're overthinking. Having not implemented it yet I'm not sure.
>>>
>>> Block commenting will look like the below
>>>
>>> /*
>>> CREATE TABLE foo (
>>>   id serial,
>>>   data text
>>> );
>>>
>>> -- Index required on data for finding Wumpus' quickly
>>> CREATE INDEX foo_idx ON foo (data);
>>> */
>>>
>>
>> Right - but how would line commenting (and uncommenting) behave. With 2
>> separate shortcuts it's pretty clear - keep adding or removing one level of
>> commenting at a time. When removing, ignore any lines without comments
>> (this is what happens now).
>>
>> If you use a single key though, what is the behaviour on the first press?
>> Comment everything, or remove the comment marker from the line in the
>> middle?
>>
>> It's a valid use-case to comment comments like that if you want to
>> disable a chunk of a script. It's clear it would work with a block comment,
>> but some folks prefer line comments for temporary use as they are much more
>> visible, especially with large blocks.
>>
>>
>>>
>>> -- Rob
>>>
>>> On Fri, Jul 21, 2017 at 11:13 AM, Dave Page  wrote:
>>>


 On Fri, Jul 21, 2017 at 4:08 PM, Robert Eckhardt 
 wrote:

> I wouldn't say wrong, it just wasn't what I was expecting.
>
> I guess I'd like to hear what others are expecting. If I had my way we
> would use
>
> Ctrl+/   single line comment and uncomment (prepend with --)
> Ctrl+Shift+/ block comment and uncomment (bracket with /* and */)
>
>
> where Ctrl == command on Mac.
>

 I think those might be easier to remember than the current keys.

 I'm not sure it makes sense to have a single key for line commenting
 and uncommenting though. Whilst it's certainly cleaner, what should the
 behaviour be for a block such as:

 CREATE TABLE foo (
   id serial,
   data text
 );

 -- Index required on data for finding Wumpus' quickly
 CREATE INDEX foo_idx ON foo (data);

 Is that likely to be a problem, or am I overthinking it?


>
> -- Rob
>
> On Fri, Jul 21, 2017 at 7:09 AM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Robert,
>>
>> I mean rather than using cmd key for Mac and CTRL key for
>> Windows/Linux, I used "CTRL" key for all the platforms.
>> And regarding choosing comma & period keys, they all are near each to
>> each other so user can remember them easily.
>>
>> Let me know If my thinking was wrong for shortcut keys, I'll change
>> them accordingly and send new patch.
>>
>> On Fri, Jul 21, 2017 at 4:25 PM, Robert Eckhardt <
>> reckha...@pivotal.io> wrote:
>>
>>> I'm not sure what you mean by across platforms.  Do you mean that
>>> those are the keyboard shortcuts in pgAdmin 3?
>>>
>>> Rob
>>>
>>> On Jul 21, 2017 4:42 AM, "Murtuza Zabuawala" <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
>>> Hi Robert,
>>>
>>> Just to make shortcut keys uniform across all the platforms.
>>>
>>> On Fri, Jul 21, 2017 at 1:25 AM, Robert Eckhardt <
>>> reckha...@pivotal.io> wrote:
>>>
 Murtuza,

 Is there a particular reason you choose the keyboard shortcuts that
 you choose. When we were looking at this earlier to see what was being 
 used
 elsewhere we discovered:

 jetbrains  cmd+/
 pycharmcmd+/
 SublimeCtrl+/ Toggle line comment
Ctrl+Shift+/ Toggle block comment
 EclipseCTRL + /
 Notepad++  CTRL+Q   Toggle line 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-21 Thread Dave Page
Thanks, patch applied and yarn.lock updated.

On Fri, Jul 21, 2017 at 3:52 PM, Sarah McAlear  wrote:

> Hi!
>
> We looked at the patch and realized that the function to check if an
> element is not clickable because it's out of view was using an incorrect
> assertion. We changed it and created a small patch. We also realized that
> on the current master the yarn.lock is causing changes when running the
> app. It looks like maybe eclint was added to package.json and it and its
> corresponding dependencies were not added to the checked in yarn.lock
> file.
>
> Thanks,
> Shruti & Sarah
>
>
>
> On Fri, Jul 21, 2017 at 4:09 AM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi Robert,
>>
>> I mean rather than using cmd key for Mac and CTRL key for Windows/Linux,
>> I used "CTRL" key for all the platforms.
>> And regarding choosing comma & period keys, they all are near each to
>> each other so user can remember them easily.
>>
>> Let me know If my thinking was wrong for shortcut keys, I'll change them
>> accordingly and send new patch.
>>
>> On Fri, Jul 21, 2017 at 4:25 PM, Robert Eckhardt 
>> wrote:
>>
>>> I'm not sure what you mean by across platforms.  Do you mean that those
>>> are the keyboard shortcuts in pgAdmin 3?
>>>
>>> Rob
>>>
>>> On Jul 21, 2017 4:42 AM, "Murtuza Zabuawala" <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
>>> Hi Robert,
>>>
>>> Just to make shortcut keys uniform across all the platforms.
>>>
>>> On Fri, Jul 21, 2017 at 1:25 AM, Robert Eckhardt 
>>> wrote:
>>>
 Murtuza,

 Is there a particular reason you choose the keyboard shortcuts that you
 choose. When we were looking at this earlier to see what was being used
 elsewhere we discovered:

 jetbrains  cmd+/
 pycharmcmd+/
 SublimeCtrl+/ Toggle line comment
Ctrl+Shift+/ Toggle block comment
 EclipseCTRL + /
 Notepad++  CTRL+Q   Toggle line comment
CTRL+SHIFT+Q Toggle block comment
 TextWrangler   Ctrl+/

 -- Rob


 On Thu, Jul 20, 2017 at 1:05 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

>
> On Thu, Jul 20, 2017 at 10:29 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> Please find patch attached, There were two issues,
>>> 1) We removed the default button to clear the editor window, it
>>> broke _clear_query_tool() functionality.
>>> 2) The buttons arrangements, we added new Edit button in between
>>> Delete and Filter button causing the "Explain" -> "Explain Options" sub
>>> menu to go out of browser visibility in feature test, so it was failing.
>>>
>>> I have put Edit button near Clear button for now, until we come up
>>> with new design for our editor for displaying these options.
>>>
>>
>> Hmm, I moved it there intentionally as it's a more traditional
>> position and thus more discoverable.
>>
>> Can we just launch the browser with a wider size, say, 1280? It's on
>> line 43 of app_starter.py...
>>
>>
> Yes, that will work too.
>
>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 I am working on this, will send you patch soon.

 On Thu, Jul 20, 2017 at 5:53 PM, Dave Page 
 wrote:

> Did you get a chance to look at this yet Murtuza?
>
> On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Sure, Will take a look.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page 
>> wrote:
>>
>>> Except I managed to break a couple of tests :-(. Can you take a
>>> look please? I've had some other work come up that I need to deal 
>>> with.
>>>
>>> 
>>> ==
>>> ERROR: runTest (pgadmin.feature_tests.query_t
>>> ool_journey_test.QueryToolJourneyTest)
>>> Tests the path through the query tool
>>> 
>>> --
>>> Traceback (most recent call last):
>>>   File "/Users/dpage/git/pgadmin4/web

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-21 Thread Dave Page
On Fri, Jul 21, 2017 at 4:08 PM, Robert Eckhardt 
wrote:

> I wouldn't say wrong, it just wasn't what I was expecting.
>
> I guess I'd like to hear what others are expecting. If I had my way we
> would use
>
> Ctrl+/   single line comment and uncomment (prepend with --)
> Ctrl+Shift+/ block comment and uncomment (bracket with /* and */)
>
>
> where Ctrl == command on Mac.
>

I think those might be easier to remember than the current keys.

I'm not sure it makes sense to have a single key for line commenting and
uncommenting though. Whilst it's certainly cleaner, what should the
behaviour be for a block such as:

CREATE TABLE foo (
  id serial,
  data text
);

-- Index required on data for finding Wumpus' quickly
CREATE INDEX foo_idx ON foo (data);

Is that likely to be a problem, or am I overthinking it?


>
> -- Rob
>
> On Fri, Jul 21, 2017 at 7:09 AM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi Robert,
>>
>> I mean rather than using cmd key for Mac and CTRL key for Windows/Linux,
>> I used "CTRL" key for all the platforms.
>> And regarding choosing comma & period keys, they all are near each to
>> each other so user can remember them easily.
>>
>> Let me know If my thinking was wrong for shortcut keys, I'll change them
>> accordingly and send new patch.
>>
>> On Fri, Jul 21, 2017 at 4:25 PM, Robert Eckhardt 
>> wrote:
>>
>>> I'm not sure what you mean by across platforms.  Do you mean that those
>>> are the keyboard shortcuts in pgAdmin 3?
>>>
>>> Rob
>>>
>>> On Jul 21, 2017 4:42 AM, "Murtuza Zabuawala" <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
>>> Hi Robert,
>>>
>>> Just to make shortcut keys uniform across all the platforms.
>>>
>>> On Fri, Jul 21, 2017 at 1:25 AM, Robert Eckhardt 
>>> wrote:
>>>
 Murtuza,

 Is there a particular reason you choose the keyboard shortcuts that you
 choose. When we were looking at this earlier to see what was being used
 elsewhere we discovered:

 jetbrains  cmd+/
 pycharmcmd+/
 SublimeCtrl+/ Toggle line comment
Ctrl+Shift+/ Toggle block comment
 EclipseCTRL + /
 Notepad++  CTRL+Q   Toggle line comment
CTRL+SHIFT+Q Toggle block comment
 TextWrangler   Ctrl+/

 -- Rob


 On Thu, Jul 20, 2017 at 1:05 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

>
> On Thu, Jul 20, 2017 at 10:29 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> Please find patch attached, There were two issues,
>>> 1) We removed the default button to clear the editor window, it
>>> broke _clear_query_tool() functionality.
>>> 2) The buttons arrangements, we added new Edit button in between
>>> Delete and Filter button causing the "Explain" -> "Explain Options" sub
>>> menu to go out of browser visibility in feature test, so it was failing.
>>>
>>> I have put Edit button near Clear button for now, until we come up
>>> with new design for our editor for displaying these options.
>>>
>>
>> Hmm, I moved it there intentionally as it's a more traditional
>> position and thus more discoverable.
>>
>> Can we just launch the browser with a wider size, say, 1280? It's on
>> line 43 of app_starter.py...
>>
>>
> Yes, that will work too.
>
>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 I am working on this, will send you patch soon.

 On Thu, Jul 20, 2017 at 5:53 PM, Dave Page 
 wrote:

> Did you get a chance to look at this yet Murtuza?
>
> On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Sure, Will take a look.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page 
>> wrote:
>>
>>> Except I managed to break a couple of tests :-(. Can you take a
>>> look please? I've had some other work come up that I need to deal 
>>> with.
>>>
>>> 
>>> ==
>>> ERROR: runTest (pgadmin.feature_tests.query_t
>>> ool_journey_test.QueryToolJourneyTest)

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-21 Thread Robert Eckhardt
I wouldn't say wrong, it just wasn't what I was expecting.

I guess I'd like to hear what others are expecting. If I had my way we
would use

Ctrl+/   single line comment and uncomment (prepend with --)
Ctrl+Shift+/ block comment and uncomment (bracket with /* and */)


where Ctrl == command on Mac.

-- Rob

On Fri, Jul 21, 2017 at 7:09 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Robert,
>
> I mean rather than using cmd key for Mac and CTRL key for Windows/Linux, I
> used "CTRL" key for all the platforms.
> And regarding choosing comma & period keys, they all are near each to each
> other so user can remember them easily.
>
> Let me know If my thinking was wrong for shortcut keys, I'll change them
> accordingly and send new patch.
>
> On Fri, Jul 21, 2017 at 4:25 PM, Robert Eckhardt 
> wrote:
>
>> I'm not sure what you mean by across platforms.  Do you mean that those
>> are the keyboard shortcuts in pgAdmin 3?
>>
>> Rob
>>
>> On Jul 21, 2017 4:42 AM, "Murtuza Zabuawala" <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>> Hi Robert,
>>
>> Just to make shortcut keys uniform across all the platforms.
>>
>> On Fri, Jul 21, 2017 at 1:25 AM, Robert Eckhardt 
>> wrote:
>>
>>> Murtuza,
>>>
>>> Is there a particular reason you choose the keyboard shortcuts that you
>>> choose. When we were looking at this earlier to see what was being used
>>> elsewhere we discovered:
>>>
>>> jetbrains  cmd+/
>>> pycharmcmd+/
>>> SublimeCtrl+/ Toggle line comment
>>>Ctrl+Shift+/ Toggle block comment
>>> EclipseCTRL + /
>>> Notepad++  CTRL+Q   Toggle line comment
>>>CTRL+SHIFT+Q Toggle block comment
>>> TextWrangler   Ctrl+/
>>>
>>> -- Rob
>>>
>>>
>>> On Thu, Jul 20, 2017 at 1:05 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>

 On Thu, Jul 20, 2017 at 10:29 PM, Dave Page  wrote:

> Hi
>
> On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Please find patch attached, There were two issues,
>> 1) We removed the default button to clear the editor window, it
>> broke _clear_query_tool() functionality.
>> 2) The buttons arrangements, we added new Edit button in between
>> Delete and Filter button causing the "Explain" -> "Explain Options" sub
>> menu to go out of browser visibility in feature test, so it was failing.
>>
>> I have put Edit button near Clear button for now, until we come up
>> with new design for our editor for displaying these options.
>>
>
> Hmm, I moved it there intentionally as it's a more traditional
> position and thus more discoverable.
>
> Can we just launch the browser with a wider size, say, 1280? It's on
> line 43 of app_starter.py...
>
>
 Yes, that will work too.

>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> I am working on this, will send you patch soon.
>>>
>>> On Thu, Jul 20, 2017 at 5:53 PM, Dave Page 
>>> wrote:
>>>
 Did you get a chance to look at this yet Murtuza?

 On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Sure, Will take a look.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page 
> wrote:
>
>> Except I managed to break a couple of tests :-(. Can you take a
>> look please? I've had some other work come up that I need to deal 
>> with.
>>
>> 
>> ==
>> ERROR: runTest (pgadmin.feature_tests.query_t
>> ool_journey_test.QueryToolJourneyTest)
>> Tests the path through the query tool
>> 
>> --
>> Traceback (most recent call last):
>>   File "/Users/dpage/git/pgadmin4/web
>> /pgadmin/feature_tests/query_tool_journey_test.py", line 45, in
>> runTest
>> self._test_history_tab()
>>   File "/Users/dpage/git/pgadmin4/web
>> /pgadmin/feature_tests/query_tool_journey_test.py", line 71, in
>> _test_history_tab
>> self.__clear_query_tool()
>>   File "/Users/dpage/git/pgadmin4/web
>> 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-21 Thread Sarah McAlear
Hi!

We looked at the patch and realized that the function to check if an
element is not clickable because it's out of view was using an incorrect
assertion. We changed it and created a small patch. We also realized that
on the current master the yarn.lock is causing changes when running the
app. It looks like maybe eclint was added to package.json and it and its
corresponding dependencies were not added to the checked in yarn.lock file.

Thanks,
Shruti & Sarah



On Fri, Jul 21, 2017 at 4:09 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Robert,
>
> I mean rather than using cmd key for Mac and CTRL key for Windows/Linux, I
> used "CTRL" key for all the platforms.
> And regarding choosing comma & period keys, they all are near each to each
> other so user can remember them easily.
>
> Let me know If my thinking was wrong for shortcut keys, I'll change them
> accordingly and send new patch.
>
> On Fri, Jul 21, 2017 at 4:25 PM, Robert Eckhardt 
> wrote:
>
>> I'm not sure what you mean by across platforms.  Do you mean that those
>> are the keyboard shortcuts in pgAdmin 3?
>>
>> Rob
>>
>> On Jul 21, 2017 4:42 AM, "Murtuza Zabuawala" <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>> Hi Robert,
>>
>> Just to make shortcut keys uniform across all the platforms.
>>
>> On Fri, Jul 21, 2017 at 1:25 AM, Robert Eckhardt 
>> wrote:
>>
>>> Murtuza,
>>>
>>> Is there a particular reason you choose the keyboard shortcuts that you
>>> choose. When we were looking at this earlier to see what was being used
>>> elsewhere we discovered:
>>>
>>> jetbrains  cmd+/
>>> pycharmcmd+/
>>> SublimeCtrl+/ Toggle line comment
>>>Ctrl+Shift+/ Toggle block comment
>>> EclipseCTRL + /
>>> Notepad++  CTRL+Q   Toggle line comment
>>>CTRL+SHIFT+Q Toggle block comment
>>> TextWrangler   Ctrl+/
>>>
>>> -- Rob
>>>
>>>
>>> On Thu, Jul 20, 2017 at 1:05 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>

 On Thu, Jul 20, 2017 at 10:29 PM, Dave Page  wrote:

> Hi
>
> On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Please find patch attached, There were two issues,
>> 1) We removed the default button to clear the editor window, it
>> broke _clear_query_tool() functionality.
>> 2) The buttons arrangements, we added new Edit button in between
>> Delete and Filter button causing the "Explain" -> "Explain Options" sub
>> menu to go out of browser visibility in feature test, so it was failing.
>>
>> I have put Edit button near Clear button for now, until we come up
>> with new design for our editor for displaying these options.
>>
>
> Hmm, I moved it there intentionally as it's a more traditional
> position and thus more discoverable.
>
> Can we just launch the browser with a wider size, say, 1280? It's on
> line 43 of app_starter.py...
>
>
 Yes, that will work too.

>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> I am working on this, will send you patch soon.
>>>
>>> On Thu, Jul 20, 2017 at 5:53 PM, Dave Page 
>>> wrote:
>>>
 Did you get a chance to look at this yet Murtuza?

 On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Sure, Will take a look.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page 
> wrote:
>
>> Except I managed to break a couple of tests :-(. Can you take a
>> look please? I've had some other work come up that I need to deal 
>> with.
>>
>> 
>> ==
>> ERROR: runTest (pgadmin.feature_tests.query_t
>> ool_journey_test.QueryToolJourneyTest)
>> Tests the path through the query tool
>> 
>> --
>> Traceback (most recent call last):
>>   File "/Users/dpage/git/pgadmin4/web
>> /pgadmin/feature_tests/query_tool_journey_test.py", line 45, in
>> runTest
>> self._test_history_tab()
>>   File "/Users/dpage/git/pgadmin4/web
>> /pgadmin/feature_tests/query_tool_journey_test.py", line 71, in
>> 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-21 Thread Robert Eckhardt
I'm not sure what you mean by across platforms.  Do you mean that those are
the keyboard shortcuts in pgAdmin 3?

Rob

On Jul 21, 2017 4:42 AM, "Murtuza Zabuawala" <
murtuza.zabuaw...@enterprisedb.com> wrote:

Hi Robert,

Just to make shortcut keys uniform across all the platforms.

On Fri, Jul 21, 2017 at 1:25 AM, Robert Eckhardt 
wrote:

> Murtuza,
>
> Is there a particular reason you choose the keyboard shortcuts that you
> choose. When we were looking at this earlier to see what was being used
> elsewhere we discovered:
>
> jetbrains  cmd+/
> pycharmcmd+/
> SublimeCtrl+/ Toggle line comment
>Ctrl+Shift+/ Toggle block comment
> EclipseCTRL + /
> Notepad++  CTRL+Q   Toggle line comment
>CTRL+SHIFT+Q Toggle block comment
> TextWrangler   Ctrl+/
>
> -- Rob
>
>
> On Thu, Jul 20, 2017 at 1:05 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>>
>> On Thu, Jul 20, 2017 at 10:29 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 Please find patch attached, There were two issues,
 1) We removed the default button to clear the editor window, it
 broke _clear_query_tool() functionality.
 2) The buttons arrangements, we added new Edit button in between Delete
 and Filter button causing the "Explain" -> "Explain Options" sub menu to go
 out of browser visibility in feature test, so it was failing.

 I have put Edit button near Clear button for now, until we come up with
 new design for our editor for displaying these options.

>>>
>>> Hmm, I moved it there intentionally as it's a more traditional position
>>> and thus more discoverable.
>>>
>>> Can we just launch the browser with a wider size, say, 1280? It's on
>>> line 43 of app_starter.py...
>>>
>>>
>> Yes, that will work too.
>>
>>>

 --
 Regards,
 Murtuza Zabuawala
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company

 On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> I am working on this, will send you patch soon.
>
> On Thu, Jul 20, 2017 at 5:53 PM, Dave Page  wrote:
>
>> Did you get a chance to look at this yet Murtuza?
>>
>> On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Sure, Will take a look.
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page 
>>> wrote:
>>>
 Except I managed to break a couple of tests :-(. Can you take a
 look please? I've had some other work come up that I need to deal with.

 
 ==
 ERROR: runTest (pgadmin.feature_tests.query_t
 ool_journey_test.QueryToolJourneyTest)
 Tests the path through the query tool
 
 --
 Traceback (most recent call last):
   File 
 "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
 line 45, in runTest
 self._test_history_tab()
   File 
 "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
 line 71, in _test_history_tab
 self.__clear_query_tool()
   File 
 "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
 line 91, in __clear_query_tool
 self.page.click_element(self.page.find_by_xpath("//*[@id='bt
 n-edit']"))
   File 
 "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
 line 148, in find_by_xpath
 return self.wait_for_element(lambda driver:
 driver.find_element_by_xpath(xpath))
   File 
 "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
 line 232, in wait_for_element
 return self._wait_for("element to exist", element_if_it_exists)
   File 
 "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
 line 282, in _wait_for
 "Timed out waiting for " + waiting_for_message)
   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
 ges/selenium/webdriver/support/wait.py", line 80, in until
 raise TimeoutException(message, screen, stacktrace)
 TimeoutException: Message: Timed out waiting for element to exist


 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-21 Thread Murtuza Zabuawala
Hi Dave,

Please find updated patch.

On Thu, Jul 20, 2017 at 10:35 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

>
> On Thu, Jul 20, 2017 at 10:29 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> Please find patch attached, There were two issues,
>>> 1) We removed the default button to clear the editor window, it
>>> broke _clear_query_tool() functionality.
>>> 2) The buttons arrangements, we added new Edit button in between Delete
>>> and Filter button causing the "Explain" -> "Explain Options" sub menu to go
>>> out of browser visibility in feature test, so it was failing.
>>>
>>> I have put Edit button near Clear button for now, until we come up with
>>> new design for our editor for displaying these options.
>>>
>>
>> Hmm, I moved it there intentionally as it's a more traditional position
>> and thus more discoverable.
>>
>> Can we just launch the browser with a wider size, say, 1280? It's on line
>> 43 of app_starter.py...
>>
>>
> Yes, that will work too.
>
>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 I am working on this, will send you patch soon.

 On Thu, Jul 20, 2017 at 5:53 PM, Dave Page  wrote:

> Did you get a chance to look at this yet Murtuza?
>
> On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Sure, Will take a look.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page  wrote:
>>
>>> Except I managed to break a couple of tests :-(. Can you take a look
>>> please? I've had some other work come up that I need to deal with.
>>>
>>> 
>>> ==
>>> ERROR: runTest (pgadmin.feature_tests.query_t
>>> ool_journey_test.QueryToolJourneyTest)
>>> Tests the path through the query tool
>>> 
>>> --
>>> Traceback (most recent call last):
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>> line 45, in runTest
>>> self._test_history_tab()
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>> line 71, in _test_history_tab
>>> self.__clear_query_tool()
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>> line 91, in __clear_query_tool
>>> self.page.click_element(self.page.find_by_xpath("//*[@id='bt
>>> n-edit']"))
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>> line 148, in find_by_xpath
>>> return self.wait_for_element(lambda driver:
>>> driver.find_element_by_xpath(xpath))
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>> line 232, in wait_for_element
>>> return self._wait_for("element to exist", element_if_it_exists)
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>> line 282, in _wait_for
>>> "Timed out waiting for " + waiting_for_message)
>>>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>> ges/selenium/webdriver/support/wait.py", line 80, in until
>>> raise TimeoutException(message, screen, stacktrace)
>>> TimeoutException: Message: Timed out waiting for element to exist
>>>
>>>
>>> 
>>> ==
>>> ERROR: runTest (pgadmin.feature_tests.query_t
>>> ool_tests.QueryToolFeatureTest)
>>> Query tool feature test
>>> 
>>> --
>>> Traceback (most recent call last):
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
>>> line 52, in runTest
>>> self._clear_query_tool()
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
>>> line 173, in _clear_query_tool
>>> self.page.find_by_id("btn-edit").click()
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>> line 151, in find_by_id
>>> return self.wait_for_element(lambda driver:
>>> driver.find_element_by_id(element_id))
>>>   File 
>>> 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-20 Thread Sarah McAlear
Hello,

Attached is a refactor that extracts the keyAction function in the sqlEditor
and placed it into a keyboard_shortcuts file. The extracted code is unit
tested. I did not look at the feature tests.

We are planning on diving into it a bit more but maybe not immediately. Up
next we will be pulling out the implementations of the functions that were
being triggered in the keyAction function.

-- Sarah





On Thu, Jul 20, 2017 at 12:55 PM, Robert Eckhardt 
wrote:

> Murtuza,
>
> Is there a particular reason you choose the keyboard shortcuts that you
> choose. When we were looking at this earlier to see what was being used
> elsewhere we discovered:
>
> jetbrains  cmd+/
> pycharmcmd+/
> SublimeCtrl+/ Toggle line comment
>Ctrl+Shift+/ Toggle block comment
> EclipseCTRL + /
> Notepad++  CTRL+Q   Toggle line comment
>CTRL+SHIFT+Q Toggle block comment
> TextWrangler   Ctrl+/
>
> -- Rob
>
>
> On Thu, Jul 20, 2017 at 1:05 PM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>>
>> On Thu, Jul 20, 2017 at 10:29 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 Please find patch attached, There were two issues,
 1) We removed the default button to clear the editor window, it
 broke _clear_query_tool() functionality.
 2) The buttons arrangements, we added new Edit button in between Delete
 and Filter button causing the "Explain" -> "Explain Options" sub menu to go
 out of browser visibility in feature test, so it was failing.

 I have put Edit button near Clear button for now, until we come up with
 new design for our editor for displaying these options.

>>>
>>> Hmm, I moved it there intentionally as it's a more traditional position
>>> and thus more discoverable.
>>>
>>> Can we just launch the browser with a wider size, say, 1280? It's on
>>> line 43 of app_starter.py...
>>>
>>>
>> Yes, that will work too.
>>
>>>

 --
 Regards,
 Murtuza Zabuawala
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company

 On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> I am working on this, will send you patch soon.
>
> On Thu, Jul 20, 2017 at 5:53 PM, Dave Page  wrote:
>
>> Did you get a chance to look at this yet Murtuza?
>>
>> On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Sure, Will take a look.
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page 
>>> wrote:
>>>
 Except I managed to break a couple of tests :-(. Can you take a
 look please? I've had some other work come up that I need to deal with.

 
 ==
 ERROR: runTest (pgadmin.feature_tests.query_t
 ool_journey_test.QueryToolJourneyTest)
 Tests the path through the query tool
 
 --
 Traceback (most recent call last):
   File 
 "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
 line 45, in runTest
 self._test_history_tab()
   File 
 "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
 line 71, in _test_history_tab
 self.__clear_query_tool()
   File 
 "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
 line 91, in __clear_query_tool
 self.page.click_element(self.page.find_by_xpath("//*[@id='bt
 n-edit']"))
   File 
 "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
 line 148, in find_by_xpath
 return self.wait_for_element(lambda driver:
 driver.find_element_by_xpath(xpath))
   File 
 "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
 line 232, in wait_for_element
 return self._wait_for("element to exist", element_if_it_exists)
   File 
 "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
 line 282, in _wait_for
 "Timed out waiting for " + waiting_for_message)
   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
 ges/selenium/webdriver/support/wait.py", line 80, in until
 raise TimeoutException(message, screen, stacktrace)

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-20 Thread Robert Eckhardt
Murtuza,

Is there a particular reason you choose the keyboard shortcuts that you
choose. When we were looking at this earlier to see what was being used
elsewhere we discovered:

jetbrains  cmd+/
pycharmcmd+/
SublimeCtrl+/ Toggle line comment
   Ctrl+Shift+/ Toggle block comment
EclipseCTRL + /
Notepad++  CTRL+Q   Toggle line comment
   CTRL+SHIFT+Q Toggle block comment
TextWrangler   Ctrl+/

-- Rob


On Thu, Jul 20, 2017 at 1:05 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

>
> On Thu, Jul 20, 2017 at 10:29 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> Please find patch attached, There were two issues,
>>> 1) We removed the default button to clear the editor window, it
>>> broke _clear_query_tool() functionality.
>>> 2) The buttons arrangements, we added new Edit button in between Delete
>>> and Filter button causing the "Explain" -> "Explain Options" sub menu to go
>>> out of browser visibility in feature test, so it was failing.
>>>
>>> I have put Edit button near Clear button for now, until we come up with
>>> new design for our editor for displaying these options.
>>>
>>
>> Hmm, I moved it there intentionally as it's a more traditional position
>> and thus more discoverable.
>>
>> Can we just launch the browser with a wider size, say, 1280? It's on line
>> 43 of app_starter.py...
>>
>>
> Yes, that will work too.
>
>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 I am working on this, will send you patch soon.

 On Thu, Jul 20, 2017 at 5:53 PM, Dave Page  wrote:

> Did you get a chance to look at this yet Murtuza?
>
> On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Sure, Will take a look.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page  wrote:
>>
>>> Except I managed to break a couple of tests :-(. Can you take a look
>>> please? I've had some other work come up that I need to deal with.
>>>
>>> 
>>> ==
>>> ERROR: runTest (pgadmin.feature_tests.query_t
>>> ool_journey_test.QueryToolJourneyTest)
>>> Tests the path through the query tool
>>> 
>>> --
>>> Traceback (most recent call last):
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>> line 45, in runTest
>>> self._test_history_tab()
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>> line 71, in _test_history_tab
>>> self.__clear_query_tool()
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>> line 91, in __clear_query_tool
>>> self.page.click_element(self.page.find_by_xpath("//*[@id='bt
>>> n-edit']"))
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>> line 148, in find_by_xpath
>>> return self.wait_for_element(lambda driver:
>>> driver.find_element_by_xpath(xpath))
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>> line 232, in wait_for_element
>>> return self._wait_for("element to exist", element_if_it_exists)
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>> line 282, in _wait_for
>>> "Timed out waiting for " + waiting_for_message)
>>>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>> ges/selenium/webdriver/support/wait.py", line 80, in until
>>> raise TimeoutException(message, screen, stacktrace)
>>> TimeoutException: Message: Timed out waiting for element to exist
>>>
>>>
>>> 
>>> ==
>>> ERROR: runTest (pgadmin.feature_tests.query_t
>>> ool_tests.QueryToolFeatureTest)
>>> Query tool feature test
>>> 
>>> --
>>> Traceback (most recent call last):
>>>   File 
>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
>>> line 52, in runTest
>>> self._clear_query_tool()
>>>   File 
>>> 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-20 Thread Murtuza Zabuawala
On Thu, Jul 20, 2017 at 10:29 PM, Dave Page  wrote:

> Hi
>
> On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Please find patch attached, There were two issues,
>> 1) We removed the default button to clear the editor window, it
>> broke _clear_query_tool() functionality.
>> 2) The buttons arrangements, we added new Edit button in between Delete
>> and Filter button causing the "Explain" -> "Explain Options" sub menu to go
>> out of browser visibility in feature test, so it was failing.
>>
>> I have put Edit button near Clear button for now, until we come up with
>> new design for our editor for displaying these options.
>>
>
> Hmm, I moved it there intentionally as it's a more traditional position
> and thus more discoverable.
>
> Can we just launch the browser with a wider size, say, 1280? It's on line
> 43 of app_starter.py...
>
>
Yes, that will work too.

>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> I am working on this, will send you patch soon.
>>>
>>> On Thu, Jul 20, 2017 at 5:53 PM, Dave Page  wrote:
>>>
 Did you get a chance to look at this yet Murtuza?

 On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Sure, Will take a look.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page  wrote:
>
>> Except I managed to break a couple of tests :-(. Can you take a look
>> please? I've had some other work come up that I need to deal with.
>>
>> 
>> ==
>> ERROR: runTest (pgadmin.feature_tests.query_t
>> ool_journey_test.QueryToolJourneyTest)
>> Tests the path through the query tool
>> 
>> --
>> Traceback (most recent call last):
>>   File 
>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>> line 45, in runTest
>> self._test_history_tab()
>>   File 
>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>> line 71, in _test_history_tab
>> self.__clear_query_tool()
>>   File 
>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>> line 91, in __clear_query_tool
>> self.page.click_element(self.page.find_by_xpath("//*[@id='bt
>> n-edit']"))
>>   File 
>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>> line 148, in find_by_xpath
>> return self.wait_for_element(lambda driver:
>> driver.find_element_by_xpath(xpath))
>>   File 
>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>> line 232, in wait_for_element
>> return self._wait_for("element to exist", element_if_it_exists)
>>   File 
>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>> line 282, in _wait_for
>> "Timed out waiting for " + waiting_for_message)
>>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/support/wait.py", line 80, in until
>> raise TimeoutException(message, screen, stacktrace)
>> TimeoutException: Message: Timed out waiting for element to exist
>>
>>
>> 
>> ==
>> ERROR: runTest (pgadmin.feature_tests.query_t
>> ool_tests.QueryToolFeatureTest)
>> Query tool feature test
>> 
>> --
>> Traceback (most recent call last):
>>   File 
>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
>> line 52, in runTest
>> self._clear_query_tool()
>>   File 
>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
>> line 173, in _clear_query_tool
>> self.page.find_by_id("btn-edit").click()
>>   File 
>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>> line 151, in find_by_id
>> return self.wait_for_element(lambda driver:
>> driver.find_element_by_id(element_id))
>>   File 
>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>> line 232, in wait_for_element
>> return self._wait_for("element to exist", element_if_it_exists)
>>   File 
>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-20 Thread Dave Page
Hi

On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> Please find patch attached, There were two issues,
> 1) We removed the default button to clear the editor window, it
> broke _clear_query_tool() functionality.
> 2) The buttons arrangements, we added new Edit button in between Delete
> and Filter button causing the "Explain" -> "Explain Options" sub menu to go
> out of browser visibility in feature test, so it was failing.
>
> I have put Edit button near Clear button for now, until we come up with
> new design for our editor for displaying these options.
>

Hmm, I moved it there intentionally as it's a more traditional position and
thus more discoverable.

Can we just launch the browser with a wider size, say, 1280? It's on line
43 of app_starter.py...


>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> I am working on this, will send you patch soon.
>>
>> On Thu, Jul 20, 2017 at 5:53 PM, Dave Page  wrote:
>>
>>> Did you get a chance to look at this yet Murtuza?
>>>
>>> On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Sure, Will take a look.

 --
 Regards,
 Murtuza Zabuawala
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company

 On Wed, Jul 19, 2017 at 8:00 PM, Dave Page  wrote:

> Except I managed to break a couple of tests :-(. Can you take a look
> please? I've had some other work come up that I need to deal with.
>
> ==
> ERROR: runTest (pgadmin.feature_tests.query_t
> ool_journey_test.QueryToolJourneyTest)
> Tests the path through the query tool
> --
> Traceback (most recent call last):
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 45, in runTest
> self._test_history_tab()
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 71, in _test_history_tab
> self.__clear_query_tool()
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 91, in __clear_query_tool
> self.page.click_element(self.page.find_by_xpath("//*[@id='bt
> n-edit']"))
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 148, in find_by_xpath
> return self.wait_for_element(lambda driver:
> driver.find_element_by_xpath(xpath))
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 232, in wait_for_element
> return self._wait_for("element to exist", element_if_it_exists)
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 282, in _wait_for
> "Timed out waiting for " + waiting_for_message)
>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
> ges/selenium/webdriver/support/wait.py", line 80, in until
> raise TimeoutException(message, screen, stacktrace)
> TimeoutException: Message: Timed out waiting for element to exist
>
>
> ==
> ERROR: runTest (pgadmin.feature_tests.query_t
> ool_tests.QueryToolFeatureTest)
> Query tool feature test
> --
> Traceback (most recent call last):
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
> line 52, in runTest
> self._clear_query_tool()
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
> line 173, in _clear_query_tool
> self.page.find_by_id("btn-edit").click()
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 151, in find_by_id
> return self.wait_for_element(lambda driver:
> driver.find_element_by_id(element_id))
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 232, in wait_for_element
> return self._wait_for("element to exist", element_if_it_exists)
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 282, in _wait_for
> "Timed out waiting for " + waiting_for_message)
>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
> ges/selenium/webdriver/support/wait.py", line 80, in until
> raise 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-20 Thread Sarah McAlear
Hi Surinder & Murtuza!

We are actually working on that right now. We should be sending a patch
soon.

Thanks!
Shruti & Sarah

On Thu, Jul 20, 2017 at 7:52 AM, Surinder Kumar <
surinder.ku...@enterprisedb.com> wrote:

> Hi Murtuza,
>
> Can the code related to Menu KeyEvents be moved out into a separate file
> say 'menu_actions.js' if feasible ?
>
> As the more functionality will be added to sqleditor, codemirror and its
> file menu in future, the codebase will eventually increase.
> It will ease writing jasmine test cases as well.
>
> Thanks,
> Surinder
>
> On Thu, Jul 20, 2017 at 8:03 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Please find patch attached, There were two issues,
>> 1) We removed the default button to clear the editor window, it
>> broke _clear_query_tool() functionality.
>> 2) The buttons arrangements, we added new Edit button in between Delete
>> and Filter button causing the "Explain" -> "Explain Options" sub menu to go
>> out of browser visibility in feature test, so it was failing.
>>
>> I have put Edit button near Clear button for now, until we come up with
>> new design for our editor for displaying these options.
>>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> I am working on this, will send you patch soon.
>>>
>>> On Thu, Jul 20, 2017 at 5:53 PM, Dave Page  wrote:
>>>
 Did you get a chance to look at this yet Murtuza?

 On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Sure, Will take a look.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page  wrote:
>
>> Except I managed to break a couple of tests :-(. Can you take a look
>> please? I've had some other work come up that I need to deal with.
>>
>> 
>> ==
>> ERROR: runTest (pgadmin.feature_tests.query_t
>> ool_journey_test.QueryToolJourneyTest)
>> Tests the path through the query tool
>> 
>> --
>> Traceback (most recent call last):
>>   File 
>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>> line 45, in runTest
>> self._test_history_tab()
>>   File 
>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>> line 71, in _test_history_tab
>> self.__clear_query_tool()
>>   File 
>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>> line 91, in __clear_query_tool
>> self.page.click_element(self.page.find_by_xpath("//*[@id='bt
>> n-edit']"))
>>   File 
>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>> line 148, in find_by_xpath
>> return self.wait_for_element(lambda driver:
>> driver.find_element_by_xpath(xpath))
>>   File 
>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>> line 232, in wait_for_element
>> return self._wait_for("element to exist", element_if_it_exists)
>>   File 
>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>> line 282, in _wait_for
>> "Timed out waiting for " + waiting_for_message)
>>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/support/wait.py", line 80, in until
>> raise TimeoutException(message, screen, stacktrace)
>> TimeoutException: Message: Timed out waiting for element to exist
>>
>>
>> 
>> ==
>> ERROR: runTest (pgadmin.feature_tests.query_t
>> ool_tests.QueryToolFeatureTest)
>> Query tool feature test
>> 
>> --
>> Traceback (most recent call last):
>>   File 
>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
>> line 52, in runTest
>> self._clear_query_tool()
>>   File 
>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
>> line 173, in _clear_query_tool
>> self.page.find_by_id("btn-edit").click()
>>   File 
>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>> line 151, in find_by_id
>> return self.wait_for_element(lambda driver:
>> driver.find_element_by_id(element_id))
>>   File 
>> 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-20 Thread Surinder Kumar
Hi Murtuza,

Can the code related to Menu KeyEvents be moved out into a separate file
say 'menu_actions.js' if feasible ?

As the more functionality will be added to sqleditor, codemirror and its
file menu in future, the codebase will eventually increase.
It will ease writing jasmine test cases as well.

Thanks,
Surinder

On Thu, Jul 20, 2017 at 8:03 PM, Murtuza Zabuawala  wrote:

> Hi Dave,
>
> Please find patch attached, There were two issues,
> 1) We removed the default button to clear the editor window, it
> broke _clear_query_tool() functionality.
> 2) The buttons arrangements, we added new Edit button in between Delete
> and Filter button causing the "Explain" -> "Explain Options" sub menu to go
> out of browser visibility in feature test, so it was failing.
>
> I have put Edit button near Clear button for now, until we come up with
> new design for our editor for displaying these options.
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> I am working on this, will send you patch soon.
>>
>> On Thu, Jul 20, 2017 at 5:53 PM, Dave Page  wrote:
>>
>>> Did you get a chance to look at this yet Murtuza?
>>>
>>> On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Sure, Will take a look.

 --
 Regards,
 Murtuza Zabuawala
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company

 On Wed, Jul 19, 2017 at 8:00 PM, Dave Page  wrote:

> Except I managed to break a couple of tests :-(. Can you take a look
> please? I've had some other work come up that I need to deal with.
>
> ==
> ERROR: runTest (pgadmin.feature_tests.query_t
> ool_journey_test.QueryToolJourneyTest)
> Tests the path through the query tool
> --
> Traceback (most recent call last):
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 45, in runTest
> self._test_history_tab()
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 71, in _test_history_tab
> self.__clear_query_tool()
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 91, in __clear_query_tool
> self.page.click_element(self.page.find_by_xpath("//*[@id='bt
> n-edit']"))
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 148, in find_by_xpath
> return self.wait_for_element(lambda driver:
> driver.find_element_by_xpath(xpath))
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 232, in wait_for_element
> return self._wait_for("element to exist", element_if_it_exists)
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 282, in _wait_for
> "Timed out waiting for " + waiting_for_message)
>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
> ges/selenium/webdriver/support/wait.py", line 80, in until
> raise TimeoutException(message, screen, stacktrace)
> TimeoutException: Message: Timed out waiting for element to exist
>
>
> ==
> ERROR: runTest (pgadmin.feature_tests.query_t
> ool_tests.QueryToolFeatureTest)
> Query tool feature test
> --
> Traceback (most recent call last):
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
> line 52, in runTest
> self._clear_query_tool()
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
> line 173, in _clear_query_tool
> self.page.find_by_id("btn-edit").click()
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 151, in find_by_id
> return self.wait_for_element(lambda driver:
> driver.find_element_by_id(element_id))
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 232, in wait_for_element
> return self._wait_for("element to exist", element_if_it_exists)
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 282, in _wait_for
> "Timed out waiting for " + waiting_for_message)
>   File 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-19 Thread Murtuza Zabuawala
Sure, Will take a look.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Wed, Jul 19, 2017 at 8:00 PM, Dave Page  wrote:

> Except I managed to break a couple of tests :-(. Can you take a look
> please? I've had some other work come up that I need to deal with.
>
> ==
> ERROR: runTest (pgadmin.feature_tests.query_tool_journey_test.
> QueryToolJourneyTest)
> Tests the path through the query tool
> --
> Traceback (most recent call last):
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 45, in runTest
> self._test_history_tab()
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 71, in _test_history_tab
> self.__clear_query_tool()
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 91, in __clear_query_tool
> self.page.click_element(self.page.find_by_xpath("//*[@id='
> btn-edit']"))
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 148, in find_by_xpath
> return self.wait_for_element(lambda driver:
> driver.find_element_by_xpath(xpath))
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 232, in wait_for_element
> return self._wait_for("element to exist", element_if_it_exists)
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 282, in _wait_for
> "Timed out waiting for " + waiting_for_message)
>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/selenium/webdriver/support/wait.py", line 80, in until
> raise TimeoutException(message, screen, stacktrace)
> TimeoutException: Message: Timed out waiting for element to exist
>
>
> ==
> ERROR: runTest (pgadmin.feature_tests.query_tool_tests.
> QueryToolFeatureTest)
> Query tool feature test
> --
> Traceback (most recent call last):
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
> line 52, in runTest
> self._clear_query_tool()
>   File 
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
> line 173, in _clear_query_tool
> self.page.find_by_id("btn-edit").click()
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 151, in find_by_id
> return self.wait_for_element(lambda driver: driver.find_element_by_id(
> element_id))
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 232, in wait_for_element
> return self._wait_for("element to exist", element_if_it_exists)
>   File 
> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
> line 282, in _wait_for
> "Timed out waiting for " + waiting_for_message)
>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/selenium/webdriver/support/wait.py", line 80, in until
> raise TimeoutException(message, screen, stacktrace)
> TimeoutException: Message: Timed out waiting for element to exist
>
>
> --
> Ran 9 tests in 262.111s
>
> On Wed, Jul 19, 2017 at 11:55 AM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Thank you Dave.
>>
>> On Wed, Jul 19, 2017 at 4:17 PM, Dave Page  wrote:
>>
>>> Thanks, applied.
>>>
>>> I also took the opportunity to tidy up the menus a little and add access
>>> keys for accessibility.
>>>
>>> One change I made was to make the Edit and Clear menus not have a
>>> default option - e.g. instead of a button with a drop-down next to it,
>>> they're now a single dropdown button with icon. I think this works better
>>> as there are no obvious candidates for the "default" option for those
>>> menus. I'm not overly enthusiastic about the look of those buttons though,
>>> so if anyone has a better idea how they should be styled, please yelp
>>> (CCing Chethana for his input as well)...
>>>
>>>
>>>
>>> On Wed, Jul 19, 2017 at 9:56 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Just a FYI,
 You need to run yarn bundle for this to be working as Surinder has
 moved all the CodeMirror code into bundle package.

 On Wed, Jul 19, 2017 at 2:20 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA updated patch,
> 1) Added Keyboard shortcuts to comment line, uncomment line and
> comment/uncomment block of code also added drop-down for the same.
> 2) Also added options for indent & unindent code in the same drop-down.
> 3) Updated 

Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-19 Thread Murtuza Zabuawala
Thank you Dave.

On Wed, Jul 19, 2017 at 4:17 PM, Dave Page  wrote:

> Thanks, applied.
>
> I also took the opportunity to tidy up the menus a little and add access
> keys for accessibility.
>
> One change I made was to make the Edit and Clear menus not have a default
> option - e.g. instead of a button with a drop-down next to it, they're now
> a single dropdown button with icon. I think this works better as there are
> no obvious candidates for the "default" option for those menus. I'm not
> overly enthusiastic about the look of those buttons though, so if anyone
> has a better idea how they should be styled, please yelp (CCing Chethana
> for his input as well)...
>
>
>
> On Wed, Jul 19, 2017 at 9:56 AM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Just a FYI,
>> You need to run yarn bundle for this to be working as Surinder has moved
>> all the CodeMirror code into bundle package.
>>
>> On Wed, Jul 19, 2017 at 2:20 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> PFA updated patch,
>>> 1) Added Keyboard shortcuts to comment line, uncomment line and
>>> comment/uncomment block of code also added drop-down for the same.
>>> 2) Also added options for indent & unindent code in the same drop-down.
>>> 3) Updated shortcut documents accordingly.
>>>
>>> Please review.
>>>
>>> On Mon, Jul 17, 2017 at 3:05 PM, Dave Page  wrote:
>>>
 Hi

 On Mon, Jul 17, 2017 at 10:31 AM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Mon, Jul 17, 2017 at 2:33 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Wed, Jul 12, 2017 at 1:16 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> PFA patch which will add functionality to allow user to
>>> comment/uncomment code in query editor.
>>> RM#2456
>>>
>>
>> This is cool, but I'm not sure it's right as-is:
>>
>> * I prefer SQL style commenting, e.g.
>>
>> -- This is a comment
>>
>> Should we make that a config option if CodeMirror can do it? Or a
>> different hotkey?
>>
>>
> I'll check the extension code and update you accordingly, and It will
> be good idea to keep the both the options because with large code block
> current style works the best.
>

 Right.


> * You've added it as an option to the Clear XXX dropdown, which really
>> isn't the right place in my opinion. Should we add a new drop down for
>> this, and include some/all of the other Editing options on there? E.g.
>> tab/shift-tab.
>>
>> I thought that is misc options dropdown for our editor, but I don't
> see any point adding new drop down for one single option, Can we add new
> button instead?
>

 I think you missed this bit:  "and include some/all of the other
 Editing options on there? E.g. tab/shift-tab.". Essentially, we'd be adding
 an Edit menu...


> * I think the docs should say Ctrl+Shift+/ rather than Shift+Ctrl+/,
>> and be ordered in the table to reflect that. It seems more natural to me.
>>
>> Initially I wrote ctrl + shift + /only but when I saw all other
> shortcuts starts with Shift , then I changed it to shift + ctrl + / :)
>

 No they don't - Ctrl+Alt+Left for example. I believe it's normal to put
 Ctrl first, then Shift as it's a modifier.


> Thoughts?
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>


 --
 Dave Page
 Blog: http://pgsnake.blogspot.com
 Twitter: @pgsnake

 EnterpriseDB UK: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company

>>>
>>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-19 Thread Dave Page
Thanks, applied.

I also took the opportunity to tidy up the menus a little and add access
keys for accessibility.

One change I made was to make the Edit and Clear menus not have a default
option - e.g. instead of a button with a drop-down next to it, they're now
a single dropdown button with icon. I think this works better as there are
no obvious candidates for the "default" option for those menus. I'm not
overly enthusiastic about the look of those buttons though, so if anyone
has a better idea how they should be styled, please yelp (CCing Chethana
for his input as well)...



On Wed, Jul 19, 2017 at 9:56 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Just a FYI,
> You need to run yarn bundle for this to be working as Surinder has moved
> all the CodeMirror code into bundle package.
>
> On Wed, Jul 19, 2017 at 2:20 PM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA updated patch,
>> 1) Added Keyboard shortcuts to comment line, uncomment line and
>> comment/uncomment block of code also added drop-down for the same.
>> 2) Also added options for indent & unindent code in the same drop-down.
>> 3) Updated shortcut documents accordingly.
>>
>> Please review.
>>
>> On Mon, Jul 17, 2017 at 3:05 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Mon, Jul 17, 2017 at 10:31 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 On Mon, Jul 17, 2017 at 2:33 PM, Dave Page  wrote:

> Hi
>
> On Wed, Jul 12, 2017 at 1:16 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch which will add functionality to allow user to
>> comment/uncomment code in query editor.
>> RM#2456
>>
>
> This is cool, but I'm not sure it's right as-is:
>
> * I prefer SQL style commenting, e.g.
>
> -- This is a comment
>
> Should we make that a config option if CodeMirror can do it? Or a
> different hotkey?
>
>
 I'll check the extension code and update you accordingly, and It will
 be good idea to keep the both the options because with large code block
 current style works the best.

>>>
>>> Right.
>>>
>>>
 * You've added it as an option to the Clear XXX dropdown, which really
> isn't the right place in my opinion. Should we add a new drop down for
> this, and include some/all of the other Editing options on there? E.g.
> tab/shift-tab.
>
> I thought that is misc options dropdown for our editor, but I don't
 see any point adding new drop down for one single option, Can we add new
 button instead?

>>>
>>> I think you missed this bit:  "and include some/all of the other
>>> Editing options on there? E.g. tab/shift-tab.". Essentially, we'd be adding
>>> an Edit menu...
>>>
>>>
 * I think the docs should say Ctrl+Shift+/ rather than Shift+Ctrl+/,
> and be ordered in the table to reflect that. It seems more natural to me.
>
> Initially I wrote ctrl + shift + /only but when I saw all other
 shortcuts starts with Shift , then I changed it to shift + ctrl + / :)

>>>
>>> No they don't - Ctrl+Alt+Left for example. I believe it's normal to put
>>> Ctrl first, then Shift as it's a modifier.
>>>
>>>
 Thoughts?
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-19 Thread Murtuza Zabuawala
Just a FYI,
You need to run yarn bundle for this to be working as Surinder has moved
all the CodeMirror code into bundle package.

On Wed, Jul 19, 2017 at 2:20 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA updated patch,
> 1) Added Keyboard shortcuts to comment line, uncomment line and
> comment/uncomment block of code also added drop-down for the same.
> 2) Also added options for indent & unindent code in the same drop-down.
> 3) Updated shortcut documents accordingly.
>
> Please review.
>
> On Mon, Jul 17, 2017 at 3:05 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Mon, Jul 17, 2017 at 10:31 AM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Mon, Jul 17, 2017 at 2:33 PM, Dave Page  wrote:
>>>
 Hi

 On Wed, Jul 12, 2017 at 1:16 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch which will add functionality to allow user to
> comment/uncomment code in query editor.
> RM#2456
>

 This is cool, but I'm not sure it's right as-is:

 * I prefer SQL style commenting, e.g.

 -- This is a comment

 Should we make that a config option if CodeMirror can do it? Or a
 different hotkey?


>>> I'll check the extension code and update you accordingly, and It will be
>>> good idea to keep the both the options because with large code block
>>> current style works the best.
>>>
>>
>> Right.
>>
>>
>>> * You've added it as an option to the Clear XXX dropdown, which really
 isn't the right place in my opinion. Should we add a new drop down for
 this, and include some/all of the other Editing options on there? E.g.
 tab/shift-tab.

 I thought that is misc options dropdown for our editor, but I don't see
>>> any point adding new drop down for one single option, Can we add new button
>>> instead?
>>>
>>
>> I think you missed this bit:  "and include some/all of the other Editing
>> options on there? E.g. tab/shift-tab.". Essentially, we'd be adding an Edit
>> menu...
>>
>>
>>> * I think the docs should say Ctrl+Shift+/ rather than Shift+Ctrl+/, and
 be ordered in the table to reflect that. It seems more natural to me.

 Initially I wrote ctrl + shift + /only but when I saw all other
>>> shortcuts starts with Shift , then I changed it to shift + ctrl + / :)
>>>
>>
>> No they don't - Ctrl+Alt+Left for example. I believe it's normal to put
>> Ctrl first, then Shift as it's a modifier.
>>
>>
>>> Thoughts?

 --
 Dave Page
 Blog: http://pgsnake.blogspot.com
 Twitter: @pgsnake

 EnterpriseDB UK: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company

>>>
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>


Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-17 Thread Murtuza Zabuawala
Hi Dave,

On Mon, Jul 17, 2017 at 2:33 PM, Dave Page  wrote:

> Hi
>
> On Wed, Jul 12, 2017 at 1:16 PM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch which will add functionality to allow user to comment/uncomment
>> code in query editor.
>> RM#2456
>>
>
> This is cool, but I'm not sure it's right as-is:
>
> * I prefer SQL style commenting, e.g.
>
> -- This is a comment
>
> Should we make that a config option if CodeMirror can do it? Or a
> different hotkey?
>
>
I'll check the extension code and update you accordingly, and It will be
good idea to keep the both the options because with large code block
current style works the best.

> * You've added it as an option to the Clear XXX dropdown, which really
> isn't the right place in my opinion. Should we add a new drop down for
> this, and include some/all of the other Editing options on there? E.g.
> tab/shift-tab.
>
> I thought that is misc options dropdown for our editor, but I don't see
any point adding new drop down for one single option, Can we add new button
instead?

> * I think the docs should say Ctrl+Shift+/ rather than Shift+Ctrl+/, and
> be ordered in the table to reflect that. It seems more natural to me.
>
> Initially I wrote ctrl + shift + /only but when I saw all other shortcuts
starts with Shift , then I changed it to shift + ctrl + / :)

> Thoughts?
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-12 Thread Robert Eckhardt
Yay.

-- Rob

On Wed, Jul 12, 2017 at 8:16 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch which will add functionality to allow user to comment/uncomment
> code in query editor.
> RM#2456
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


[pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

2017-07-12 Thread Murtuza Zabuawala
Hi,

PFA patch which will add functionality to allow user to comment/uncomment
code in query editor.
RM#2456

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/docs/en_US/keyboard_shortcuts.rst 
b/docs/en_US/keyboard_shortcuts.rst
index 2194d63..9de6103 100644
--- a/docs/en_US/keyboard_shortcuts.rst
+++ b/docs/en_US/keyboard_shortcuts.rst
@@ -59,6 +59,8 @@ When using the syntax-highlighting SQL editors, the following 
shortcuts are avai
 
+--+--+-+
 | Shift+Tab| Shift+Tab| Un-indent selected text
 |
 
+--+--+-+
+| Shift+Ctrl+/ | Shift+Ctrl+/ | Comment/Uncomment selected 
text |
++--+--+-+
 
 **Query Tool**
 
diff --git a/web/pgadmin/tools/datagrid/templates/datagrid/index.html 
b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
index 4bb38c4..b9fe89d 100644
--- a/web/pgadmin/tools/datagrid/templates/datagrid/index.html
+++ b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
@@ -217,6 +217,10 @@
 
 
 
+
+
+ {{ _('Comment/Uncomment Code') }} 
+
 
 
  {{ _('Clear Query Window') }} 
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js 
b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 8a797ab..3cf9fc6 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
@@ -28,6 +28,7 @@ define([
 'codemirror/addon/search/jump-to-line',
 'codemirror/addon/edit/matchbrackets',
 'codemirror/addon/edit/closebrackets',
+'codemirror/addon/comment/comment',
 
 'backgrid.sizeable.columns',
 'slick.pgadmin.formatters',
@@ -50,7 +51,8 @@ define([
 // Define key codes for shortcut keys
 var F5_KEY = 116,
 F7_KEY = 118,
-F8_KEY = 119;
+F8_KEY = 119,
+FWD_SLASH_KEY = 191;
 
 var is_query_running = false;
 
@@ -102,6 +104,7 @@ define([
 "click #btn-explain-costs": "on_explain_costs",
 "click #btn-explain-buffers": "on_explain_buffers",
 "click #btn-explain-timing": "on_explain_timing",
+"click #btn-toggle-comment": "on_toggle_comment_code",
 "change .limit": "on_limit_change",
 "keydown": "keyAction"
   },
@@ -1288,6 +1291,17 @@ define([
 );
   },
 
+  // Callback function for the comment/uncomment selected code
+  on_toggle_comment_code: function() {
+var self = this;
+// Trigger the comment signal to the SqlEditorController class
+self.handler.trigger(
+'pgadmin-sqleditor:toggle_comment_code',
+self,
+self.handler
+);
+  },
+
   // Callback function for the clear button click.
   on_clear: function(ev) {
 var self = this, sql;
@@ -1498,6 +1512,10 @@ define([
   // Download query result as CSV.
   this.on_download(ev);
   ev.preventDefault();
+} else if (ev.shiftKey && ev.ctrlKey  && keyCode == FWD_SLASH_KEY ) {
+  // Toggle comments
+  this.on_toggle_comment_code(ev);
+  ev.preventDefault();
 }
   }
 });
@@ -1587,6 +1605,8 @@ define([
   self.on('pgadmin-sqleditor:button:explain-costs', 
self._explain_costs, self);
   self.on('pgadmin-sqleditor:button:explain-buffers', 
self._explain_buffers, self);
   self.on('pgadmin-sqleditor:button:explain-timing', 
self._explain_timing, self);
+  self.on('pgadmin-sqleditor:toggle_comment_code', 
self._toggle_comment_code, self);
+
 
   if (self.is_query_tool) {
 self.gridView.query_tool_obj.refresh();
@@ -3635,6 +3655,25 @@ define([
 },
 
 /*
+ * This function will comment/uncomment code
+ */
+_toggle_comment_code: function() {
+  var self = this, editor = self.gridView.query_tool_obj,
+  selected_code = editor.getSelection(),
+  sql = selected_code.length > 0 ? selected_code : editor.getValue();
+  // If it is an empty query, do nothing.
+  if (sql.length <= 0) return;
+
+  // Find the code selection range
+  var range = {
+from: editor.getCursor(true),
+to: editor.getCursor(false)
+  };
+  // Comment/Uncomment code
+  editor.toggleComment(range.from, range.to);
+},
+
+/*
  * This function get explain options and auto rollback/auto commit
  * values