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/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
