Re: [pgAdmin4][Debugger]: Initial Patch

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Debugger]: Initial Patch
Date: 2016-05-06 12:20:13
Message-ID: CANxoLDcvq-jPi1u+hOjpLiida=+t5h+5j3fxL+QSCmj9gxLxzw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Mon, Apr 25, 2016 at 7:18 PM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
wrote:

> Hi,
>
> Please find attached patch file with below fixes.
>
> - Removed the "lineWrapping" option from the codemirror textarea
> because it was creating issue in the code folding.
> - Handle the values while depositing during debugging.
> - Properly handle the Array values while saving it to sqlite database
> and displayed in input dialog.
> - SQL code folding was not supported in codemirror so added the same.
> Currently we have added support for below keywords.
> IF-END IF
> LOOP-END LOOP
> BEGIN - END
> CASE- END CASE
> We will add more SQL folding support later ( e.g. Fold multiline query
> inside function, Folding of create function etc.)
>
> As query tool also requires the use of code folding so below is
> the usage for it.
> - Define below require javascript files in modules.
> 'codemirror/addon/fold/foldgutter',
> 'codemirror/addon/fold/foldcode',
> 'codemirror/addon/fold/pgadmin-sqlfoldcode'
>
> - Remove "lineWrapping" option from the codemirror textarea.
>
> - Add the below options while creating the Codemirror
> textarea.
>
> *foldOptions*: {
> widget: "\u2026"
> },
> *foldGutter*: {
> rangeFinder:
> CodeMirror.fold.combine(CodeMirror.pgadminBeginRangeFinder,
> CodeMirror.pgadminIfRangeFinder,
>
> CodeMirror.pgadminLoopRangeFinder, CodeMirror.pgadminCaseRangeFinder)
> },
> *gutters*: ["CodeMirror-foldgutter"]
>
>
> Do review it and let us know for comments.
>

Thanks patch applied.

>
> Thanks,
> Neel Patel
>
>
> On Wed, Apr 20, 2016 at 6:32 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Thanks - applied!
>>
>>
>> On Tuesday, April 19, 2016, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
>> 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 <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
>>>>
>>>>
>>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>
>

--
*Akshay Joshi*
*Principal Software Engineer *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2016-05-06 12:25:58 pgAdmin 4 commit: Added dependency of messages 'pgadmin.browser.message
Previous Message Khushboo Vashi 2016-05-06 12:17:03 []pgAdmin4[Patch]: Dependencies Tab: Type Icon Issue fix