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 <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+/ >>> pycharm cmd+/ >>> Sublime Ctrl+/ Toggle line comment >>> Ctrl+Shift+/ Toggle block comment >>> Eclipse CTRL + / >>> 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 <dp...@pgadmin.org> 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 <dp...@pgadmin.org> >>>>>>> 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 <dp...@pgadmin.org> >>>>>>>>> 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/pga >>>>>>>>>> dmin4/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_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/pga >>>>>>>>>> dmin4/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 < >>>>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>>>>>>>> >>>>>>>>>>> Thank you Dave. >>>>>>>>>>> >>>>>>>>>>> On Wed, Jul 19, 2017 at 4:17 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>>>> 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 shortcut documents accordingly. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Please review. >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Mon, Jul 17, 2017 at 3:05 PM, Dave Page <dp...@pgadmin.org >>>>>>>>>>>>>> > 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 < >>>>>>>>>>>>>>>> dp...@pgadmin.org> 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 >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> 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 >>>>> >>>> >>>> >>> >> >> >
