Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ayek1-0005Dy-Lm for pgadmin-hackers@arkaria.postgresql.org; Fri, 06 May 2016 12:20:21 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1ayek0-0008Ju-W5 for pgadmin-hackers@arkaria.postgresql.org; Fri, 06 May 2016 12:20:21 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1ayek0-0008Jn-4k for pgadmin-hackers@postgresql.org; Fri, 06 May 2016 12:20:20 +0000 Received: from mail-ob0-x229.google.com ([2607:f8b0:4003:c01::229]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1ayejv-0000xn-HN for pgadmin-hackers@postgresql.org; Fri, 06 May 2016 12:20:19 +0000 Received: by mail-ob0-x229.google.com with SMTP id dm5so50777811obc.1 for ; Fri, 06 May 2016 05:20:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=TW4P+BNEMUyiJzuI3kAyoZ8i2Ifc1VtxWm7aKZi+5/Q=; b=E69yXS93dCuHc3CFWZn/d80HBIzMFgdzV9QKmW+qcdu4V2q07AGnUiXO9twtgs76yV Vno2RaHbirlOHrWCk/nvS7UvR4bbBc813TR2lmqtDdyi662gklLsaovWfEYFp224EV5C H3y5qfyD+tE/4gdHYcLi78qytcRvkXyONSxCDHbrOd94TNZL4eY9iReR+fxyv/06DbPc PFEOvUflukUt8JRkC7dsAH1kAx5uNJovPlOHiy30LJaNEvoeRLTYPOWig7eyiYeVuAv6 ZVGY+CjsQAZ3mypIiZ24OAPjpuyoeNOjjb8dCZMESGcRXsAjPGcZpzBThZFNEZnB6T0t 3Dqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=TW4P+BNEMUyiJzuI3kAyoZ8i2Ifc1VtxWm7aKZi+5/Q=; b=MdsVqly5XVtbf9tPjcfJR0VMCs86Y4W972A7qIBoa72JTvOhTMDDSShcXZMwlHf0hv 4e7wkig0nzyn+RejhhJdA7xiC7JwBLpTEhL6s6GbGtkzl4LgEHoDLmD9Xt85jW1qPvY8 aaJlrGt1uzcU1B5mFMTFPo7HMujHAGa1LAjQCNb2uoVRSO6XRVThyqg9yNjaERMpjGbP gtlURyTr58WPmIHbP2I8Dns7VOraaaiX0lfL972XDp/acrTa+DB1qG/lFC52Xk2O3iTf 8F6e6Vo2o2d1uxJgZFLyZUo4rwt/WWG0uzdOfRGp9BW//BmJgiQbXb75/0JrX5LzCZIo +kAg== X-Gm-Message-State: AOPr4FVCJzvMze212QUSEpBZA0aIRAt7H0ImVvdgNzL5La8E25IaDXsHYn9ZgqyDBbRKWW7Fmum+3xIXKgiKJ6Ep MIME-Version: 1.0 X-Received: by 10.182.68.110 with SMTP id v14mr9420761obt.27.1462537213750; Fri, 06 May 2016 05:20:13 -0700 (PDT) Received: by 10.202.175.148 with HTTP; Fri, 6 May 2016 05:20:13 -0700 (PDT) In-Reply-To: References: Date: Fri, 6 May 2016 17:50:13 +0530 Message-ID: Subject: Re: [pgAdmin4][Debugger]: Initial Patch From: Akshay Joshi To: Neel Patel Cc: Dave Page , pgadmin-hackers Content-Type: multipart/alternative; boundary=e89a8fb2015618c6b005322b7b7a X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --e89a8fb2015618c6b005322b7b7a Content-Type: text/plain; charset=UTF-8 On Mon, Apr 25, 2016 at 7:18 PM, Neel Patel 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 wrote: > >> Thanks - applied! >> >> >> On Tuesday, April 19, 2016, Neel Patel >> 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 wrote: >>> >>>> Hi >>>> >>>> On Monday, April 18, 2016, Neel Patel >>>> 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 wrote: >>>>> >>>>>> Hi >>>>>> >>>>>> On Thu, Apr 14, 2016 at 1:52 PM, Neel Patel < >>>>>> neel.patel@enterprisedb.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@postgresql.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* --e89a8fb2015618c6b005322b7b7a Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

= On Mon, Apr 25, 2016 at 7:18 PM, Neel Patel <neel.patel@e= nterprisedb.com> wrote:
Hi,

Please find attached patch= file with below fixes.
  • Removed the "lineWrapping&qu= ot; option from the codemirror textarea because it was creating issue in th= e code folding.
  • Handle the values while depositing during debug= ging.
  • Properly handle the Array values while saving it to sqlit= e 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.
    =C2=A0 =C2=A0 =C2=A0 IF-END IF=C2=A0=
    =C2=A0 =C2=A0 =C2=A0 LOOP-END LOOP
    =C2=A0 =C2=A0 =C2= =A0 BEGIN - END
    =C2=A0 =C2=A0 =C2=A0 CASE- END CASE
    We = will add more SQL folding support later ( e.g. Fold multiline query inside = function, Folding of create function etc.)
=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0As query tool also requires the use of code = folding so below is the usage for it.
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 - Define below require javascript files= in modules.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0'codemirror/addon/fold/foldgutter',=C2=A0
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0'codemirror/= addon/fold/foldcode',=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0'codemirror/addon/fold/pgadmin-sqlfoldco= de'

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 - Remove "lineWrapping" option from the codemirror textare= a.

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 - Add the below options while creating the Codemirror textarea.

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 foldOptions: {
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 widget: = "\u2026"
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 },
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 foldGutter: {
=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 rangeFinder: CodeMirror.fold.combine(CodeMirror.pgadminBeginR= angeFinder, CodeMirror.pgadminIfRangeFinder,
=C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0CodeMirror.pgadminLoopRangeFinder, CodeMirror.pgadminCaseRangeFinder)=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0},
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0gutters: ["CodeMirror-foldgutter&quo= t;]

=C2=A0 =C2=A0
Do review it and let us know for comments.

=C2=A0 =C2=A0 Thanks patch applied.=C2=A0

Thanks,
Neel Patel


On Wed= , Apr 20, 2016 at 6:32 PM, Dave Page <dpage@pgadmin.org> wro= te:
Thanks - applied!


On= Tuesday, April 19, 2016, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi Dave,

Please find the attached patch file with below fix.
  • Re= move the duplicate CSS and used the actual class for the debugger button to= olbar.
  • As per the Ashesh's comment, previously we used 2 wcDock= er instance to arrange the multiple tabs to main debugger panel. Now with t= his patch file, we have used only 1 wcDocker instance.
Do rev= iew it and let us know for comments.

Thanks,=
Neel =C2=A0Patel

On Mon, Apr 18, 2016 at 6:07 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Monday, April 18, 2016, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi Dave,

Please find inline comments= with all the fixes.
Attached is the updated patch file. Do revie= w 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 cl= asses 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 m= inimum button widths, and duplicates CSS unnecessarily.

Thanks.
=C2=A0

On Fri, Apr 15, 2016 at 2:09 AM, Dave Page <dpage= @pgadmin.org> wrote:
Hi

On Thu, Apr 14, 2016 at 1:52 PM, Neel Patel <neel.patel@enterprisedb.= com> wrote:
> Hi,
>
> Please find attached v2 patch file of the debugger which fixes the bel= ow
> 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 value= s 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" b= utton.
> User can debug the "procedure" which is supported in PPAS da= tabase.
> Replaced the "Glyphicon" with the "font-awesome" i= cons.

Very cool! Committed, understanding that there are still improvement= s
to be made.

> Below are the TODOs
>
> Validate the input arguments values changed by user while depositing t= he
> 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 "arr= ow" next to
> breakpoint in the gutters as per the pgadmin3.
> Need to add "Debug package initializer" in the user input di= alog for the
> direct debugging.
> Last but not least "Review comments" :)

Here you go :-)

- Ensure all messages are gettext enabled.
=C2=A0
Fixed.=C2=A0

- Constructs like the following won't work, because Jinja will
evaluate the string " + err.errormsg + " before it ever gets eval= uated
as JS by the browser.

Alertify.alert("{{ _('" + err.errormsg + "') }}"= ;);
=C2=A0
Fixed.
=C2=A0

- Please adjust the button bar to use the same styling as the button
bar on the Properties tab.
=C2=A0
Fixed=C2= =A0

- 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.
=C2=A0
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.
=C2=A0

- 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 ti= meout 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.)=C2=A0 b= ut 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 wh= ere
> 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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers=




--
Aksh= ay Joshi


<= b>Phone: +91 20-3058-9517
Mobile: +91 976-788= -8246
--e89a8fb2015618c6b005322b7b7a--