From: | Neel Patel <neel(dot)patel(at)enterprisedb(dot)com> |
---|---|
To: | Dave Page <dpage(at)pgadmin(dot)org> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: [pgAdmin4][Debugger]: Initial Patch |
Date: | 2016-04-19 06:29:57 |
Message-ID: | CACCA4P26NCZhfVVn=AfM3sd1t-JvC-hzEzK84M-O8K6Vz-Jp-A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
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 <dpage(at)pgadmin(dot)org> wrote:
> Hi
>
> On Monday, April 18, 2016, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com> 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 <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi
>>>
>>> On Thu, Apr 14, 2016 at 1:52 PM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
>>> 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
>
>
Attachment | Content-Type | Size |
---|---|---|
debugger_19_April_Fixes.patch | application/octet-stream | 10.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Sanket Mehta | 2016-04-19 09:59:59 | Re: PATCH: graphical explain |
Previous Message | Murtuza Zabuawala | 2016-04-18 13:34:06 | [PATCH] Tables node (pgAdmin4) |