On Fri, Jul 21, 2017 at 4:08 PM, Robert Eckhardt <[email protected]> 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.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 <[email protected]> >> 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" < >>> [email protected]> wrote: >>> >>> Hi Robert, >>> >>> Just to make shortcut keys uniform across all the platforms. >>> >>> On Fri, Jul 21, 2017 at 1:25 AM, Robert Eckhardt <[email protected]> >>> 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 < >>>> [email protected]> wrote: >>>> >>>>> >>>>> On Thu, Jul 20, 2017 at 10:29 PM, Dave Page <[email protected]> wrote: >>>>> >>>>>> Hi >>>>>> >>>>>> On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala < >>>>>> [email protected]> 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 < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> Hi Dave, >>>>>>>> >>>>>>>> I am working on this, will send you patch soon. >>>>>>>> >>>>>>>> On Thu, Jul 20, 2017 at 5:53 PM, Dave Page <[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> Did you get a chance to look at this yet Murtuza? >>>>>>>>> >>>>>>>>> On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala < >>>>>>>>> [email protected]> 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 <[email protected]> >>>>>>>>>> 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 < >>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>> >>>>>>>>>>>> Thank you Dave. >>>>>>>>>>>> >>>>>>>>>>>> On Wed, Jul 19, 2017 at 4:17 PM, Dave Page <[email protected]> >>>>>>>>>>>> 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 < >>>>>>>>>>>>> [email protected]> 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 < >>>>>>>>>>>>>> [email protected]> 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 < >>>>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Hi >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Mon, Jul 17, 2017 at 10:31 AM, Murtuza Zabuawala < >>>>>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Hi Dave, >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On Mon, Jul 17, 2017 at 2:33 PM, Dave Page < >>>>>>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Hi >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> On Wed, Jul 12, 2017 at 1:16 PM, Murtuza Zabuawala < >>>>>>>>>>>>>>>>>> [email protected]> 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 >>>>>> >>>>> >>>>> >>>> >>> >>> >> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
