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_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 <murtuza.zabuawala@ > 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 >