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.zabuawala@ > 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/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 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 >
