Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor

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 <dpage(at)pgadmin(dot)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("//*[(at)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 <dpage(at)pgadmin(dot)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(dot)zabuawala(at)enterprisedb(dot)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(dot)zabuawala(at)enterprisedb(dot)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 <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> On Mon, Jul 17, 2017 at 10:31 AM, Murtuza Zabuawala <
>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> On Mon, Jul 17, 2017 at 2:33 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Wed, Jul 12, 2017 at 1:16 PM, Murtuza Zabuawala <
>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)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
>