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

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Chethana Kumar <chethana(dot)kumar(at)enterprisedb(dot)com>
Subject: Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor
Date: 2017-07-19 14:30:32
Message-ID: CA+OCxoykm-Tr+poGpKp7nEWcFBNeAP2Jb1SCsWhr9PhMjDNUhg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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(dot)zabuawala(at)enterprisedb(dot)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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Murtuza Zabuawala 2017-07-19 14:37:20 Re: [pgAdmin4][Patch]: Allow user to Comment/Uncomment code in query editor
Previous Message pgAdmin 4 Jenkins 2017-07-19 14:27:28 Build failed in Jenkins: pgadmin4-master-python26 #369