public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dave Page <[email protected]>
To: Neel Patel <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Debugger]: Initial Patch
Date: Wed, 20 Apr 2016 09:02:15 -0400
Message-ID: <CA+OCxoxQTncHeTw-Wd=Vmoa1BcouenV00MypEOwz0sGBRJobRA@mail.gmail.com> (raw)
In-Reply-To: <CACCA4P26NCZhfVVn=AfM3sd1t-JvC-hzEzK84M-O8K6Vz-Jp-A@mail.gmail.com>
References: <CACCA4P1PzVEELk_=8NbpKtgOwVSypG--jRssRLtb8i_SWCcHtg@mail.gmail.com>
<CA+OCxoyxgw6W8rxUFU=FqeasjJUT+VE+odUGN1JeY-ZVmqNr7Q@mail.gmail.com>
<CACCA4P2bT+yHiHidnaA9poLZ8ZVLU57pJc8qArmK_nmw=YxHVg@mail.gmail.com>
<CACCA4P2FuiVH-+xJh1gC5KJAeS4yTZuqMJHHh1u1fgpHdXZFgg@mail.gmail.com>
<CACCA4P0kT0v9FuC2e_4ua0Jz2DBR8yFjeWKDQG-c--0c5KUY=A@mail.gmail.com>
<CACCA4P3Qzq=PXWhX2iG7cXNVexJO--RQaK++pE54yKzCVPXcDw@mail.gmail.com>
<CA+OCxowidzrBrk1C-Z4JyF5Hqpk43TY8RQXtPwHd5TCgBdQQ8g@mail.gmail.com>
<CACCA4P3DMJ3ArcwZ5v-AOOuHwfMJQqc-iNCeeu09XdHL3rsG1Q@mail.gmail.com>
<CA+OCxoycghjOcbiRd+sDhYFdOgADhXAc=82HZ=XKmS-CsasqhA@mail.gmail.com>
<CACCA4P3kNCbDc_BvYi69yDFc2cO44c5yoZg3LRA04REyjYJegg@mail.gmail.com>
<CA+OCxozPzu3oZxqndT9kihiCU-JjYAgyhHM_xdGaEGErkdXTLw@mail.gmail.com>
<CACCA4P26NCZhfVVn=AfM3sd1t-JvC-hzEzK84M-O8K6Vz-Jp-A@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
Thanks - applied!
On Tuesday, April 19, 2016, Neel Patel <[email protected]> wrote:
> Hi Dave,
>
> Please find the attached patch file with below fix.
>
> - Remove the duplicate CSS and used the actual class for the debugger
> button toolbar.
> - As per the Ashesh's comment, previously we used 2 wcDocker instance
> to arrange the multiple tabs to main debugger panel. Now with this patch
> file, we have used only 1 wcDocker instance.
>
> Do review it and let us know for comments.
>
> Thanks,
> Neel Patel
>
> On Mon, Apr 18, 2016 at 6:07 PM, Dave Page <[email protected]
> <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote:
>
>> Hi
>>
>> On Monday, April 18, 2016, Neel Patel <[email protected]
>> <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote:
>>
>>> Hi Dave,
>>>
>>> Please find inline comments with all the fixes.
>>> Attached is the updated patch file. Do review it and let me know for any
>>> comments.
>>>
>>> If you find any issues, let me know .Now, Working on the remaining TODOs
>>> as mentioned in below email.
>>>
>>
>> Thanks - committed with some minor tweaks. One problem partly still
>> remains though - you've partially copied the toolbar styling. Please use
>> the actual classes used by the Properties panel. I've already updated the
>> query tool in that way. Whilst your version looks much closer, it's missing
>> the minimum button widths, and duplicates CSS unnecessarily.
>>
>> Thanks.
>>
>>
>>>
>>> On Fri, Apr 15, 2016 at 2:09 AM, Dave Page <[email protected]> wrote:
>>>
>>>> Hi
>>>>
>>>> On Thu, Apr 14, 2016 at 1:52 PM, Neel Patel <
>>>> [email protected]> wrote:
>>>> > Hi,
>>>> >
>>>> > Please find attached v2 patch file of the debugger which fixes the
>>>> below
>>>> > issues which was not present in the first patch.
>>>> > In this patch, we have added new table in sqlite database to store the
>>>> > functions arguments value user has given during debugging.
>>>> > After applying this patch, user needs to execute "setup.py" to create
>>>> the
>>>> > new table in pgadmin4.db file.
>>>> >
>>>> > In direct debugging, when user debug the function then arguments
>>>> values will
>>>> > be stored in the sqlite database so when user debug the same function
>>>> again
>>>> > then previous values will be filled in the user input dialog.
>>>> > Once the execution is completed then user will be able to do the
>>>> debug of
>>>> > the same function again by pressing the "Continue/Restart" button.
>>>> > User can debug the "procedure" which is supported in PPAS database.
>>>> > Replaced the "Glyphicon" with the "font-awesome" icons.
>>>>
>>>> Very cool! Committed, understanding that there are still improvements
>>>> to be made.
>>>>
>>>> > Below are the TODOs
>>>> >
>>>> > Validate the input arguments values changed by user while depositing
>>>> the
>>>> > value during debugging.
>>>> > Need to implement the code folding in the codemirror editor area.
>>>> > As per the Ashesh's suggestion, need to add debug logs information so
>>>> that
>>>> > we can get the state of the debug function. Also need to add "arrow"
>>>> next to
>>>> > breakpoint in the gutters as per the pgadmin3.
>>>> > Need to add "Debug package initializer" in the user input dialog for
>>>> the
>>>> > direct debugging.
>>>> > Last but not least "Review comments" :)
>>>>
>>>> Here you go :-)
>>>>
>>>> - Ensure all messages are gettext enabled.
>>>>
>>>
>>> Fixed.
>>>
>>>>
>>>> - Constructs like the following won't work, because Jinja will
>>>> evaluate the string " + err.errormsg + " before it ever gets evaluated
>>>> as JS by the browser.
>>>>
>>>> Alertify.alert("{{ _('" + err.errormsg + "') }}");
>>>>
>>>
>>> Fixed.
>>>
>>>
>>>>
>>>> - Please adjust the button bar to use the same styling as the button
>>>> bar on the Properties tab.
>>>>
>>>
>>> Fixed
>>>
>>>>
>>>> - Let's make the stack pane tab part of the tabset at the bottom of
>>>> the debugger, and ensure docking etc. works so tabs can be split off
>>>> and arranged around the main source window.
>>>>
>>>
>>> Fixed. Now stack pane will be displayed along with another panel at
>>> bottom and also docking has been introduced for all the panels so tabs will
>>> be arranged around main debugger panel.
>>>
>>>
>>>>
>>>> - Stepping is quite slow. What's causing that? I wonder if we really
>>>> need to have all the one line SQL templates - they're probably quite
>>>> expensive to process.
>>>>
>>> Fixed. This is due to polling timeout was high (1 second) and we are
>>> getting delay in getting the results. Now polling timeout has reduced to to
>>> 200ms.
>>>
>>>>
>>>> - Is backend_running.sql required? I've removed both versions as I
>>>> can't find any references to them. Are any other templates not
>>>> required?
>>>>
>>> Ok. No other templates.
>>>
>>>>
>>>> Will log any other issues that come up in further work.
>>>>
>>>> > Below functionalities are implemented but testing are pending.
>>>> >
>>>> > Trigger functions need to test with the debugger.
>>>> > Functions are tested with data types (like text, integer etc.) but
>>>> it needs
>>>> > to be tested with all the data types for direct debugging.
>>>> > Functions/Procedures need to test with PPAS 9.2 and earlier version
>>>> where
>>>> > debugger version is different.
>>>>
>>>> Thanks!
>>>>
>>>> --
>>>> 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
view thread (15+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [pgAdmin4][Debugger]: Initial Patch
In-Reply-To: <CA+OCxoxQTncHeTw-Wd=Vmoa1BcouenV00MypEOwz0sGBRJobRA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox