public inbox for [email protected]  
help / color / mirror / Atom feed
From: Neel Patel <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Debugger]: Initial Patch
Date: Tue, 19 Apr 2016 11:59:57 +0530
Message-ID: <CACCA4P26NCZhfVVn=AfM3sd1t-JvC-hzEzK84M-O8K6Vz-Jp-A@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxozPzu3oZxqndT9kihiCU-JjYAgyhHM_xdGaEGErkdXTLw@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>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-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 <[email protected]> wrote:

> Hi
>
> On Monday, April 18, 2016, Neel Patel <[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
>
>


-- 
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers


Attachments:

  [application/octet-stream] debugger_19_April_Fixes.patch (10.3K, 3-debugger_19_April_Fixes.patch)
  download | inline diff:
diff --git a/web/pgadmin/tools/debugger/static/css/debugger.css b/web/pgadmin/tools/debugger/static/css/debugger.css
index dfeb331..cc0c1a5 100644
--- a/web/pgadmin/tools/debugger/static/css/debugger.css
+++ b/web/pgadmin/tools/debugger/static/css/debugger.css
@@ -1,19 +1,3 @@
-.btn-step-into, .btn-step-over, .btn-continue, .btn-toggle-breakpoint, .btn-clear-breakpoint, .btn-stop {
-  background-color: #D2D2D2;
-  left: 0px;
-  right: 0px;
-  padding: 7px;
-}
-
-.debugger-toolbar {
-  background-color: #D2D2D2;
-  border: 2px solid #A9A9A9;
-  left: 0px;
-  right: 0px;
-  padding: 0px;
-  padding-left: 2px;
-}
-
 #container {
   position: absolute;
   top: 44px;
@@ -63,10 +47,10 @@
   text-align: center;
 }
 
-.pg-debugger-panel .breakpoints {
+.debugger-container .breakpoints {
     width: 0.9em;
 }
 
-.pg-debugger-panel .CodeMirror-activeline-background {
+.debugger-container .CodeMirror-activeline-background {
     background: #50B0F0;
 }
\ No newline at end of file
diff --git a/web/pgadmin/tools/debugger/templates/debugger/direct.html b/web/pgadmin/tools/debugger/templates/debugger/direct.html
index a72b90a..e98e7d0 100644
--- a/web/pgadmin/tools/debugger/templates/debugger/direct.html
+++ b/web/pgadmin/tools/debugger/templates/debugger/direct.html
@@ -19,40 +19,34 @@ try {
 {% block body %}
 
 <nav class="navbar-inverse navbar-fixed-top">
-  <div class="container-fluid debugger-toolbar">
-    <div class="collapse navbar-collapse">
-      <ul class="nav navbar-nav">
-        <div id="btn-toolbar" class="btn-toolbar" role="toolbar" aria-label="">
-          <div class="btn-group" role="group" aria-label="">
-              <button type="button" class="btn btn-default btn-step-into" title="{{ _('Step into') }}">
-                  <i class="fa fa-indent"></i>
-              </button>
-              <button type="button" class="btn btn-default btn-step-over" title="{{ _('Step over') }}">
-                  <i class="fa fa-outdent"></i>
-              </button>
-              <button type="button" class="btn btn-default btn-continue" title="{{ _('Continue/Start') }}">
-                  <i class="fa fa-play-circle"></i>
-              </button>
-          </div>
-           <div class="btn-group" role="group" aria-label="">
-              <button type="button" class="btn btn-default btn-toggle-breakpoint" title="{{ _('Toggle breakpoint') }}">
-                  <i class="fa fa-circle"></i>
-              </button>
-              <button type="button" class="btn btn-default btn-clear-breakpoint" title="{{ _('Clear all breakpoints') }}">
-                  <i class="fa fa-ban"></i>
-              </button>
-          </div>
-           <div class="btn-group" role="group" aria-label="">
-              <button type="button" class="btn btn-default btn-stop" title="{{ _('Stop') }}">
-                  <i class="fa fa-stop-circle"></i>
-              </button>
-          </div>
+    <div id="btn-toolbar" class="btn-toolbar pg-prop-btn-group" role="toolbar" aria-label="">
+        <div class="btn-group" role="group" aria-label="">
+            <button type="button" class="btn btn-default btn-step-into" title="{{ _('Step into') }}">
+                <i class="fa fa-indent"></i>
+            </button>
+            <button type="button" class="btn btn-default btn-step-over" title="{{ _('Step over') }}">
+                <i class="fa fa-outdent"></i>
+            </button>
+            <button type="button" class="btn btn-default btn-continue" title="{{ _('Continue/Start') }}">
+                <i class="fa fa-play-circle"></i>
+            </button>
+        </div>
+        <div class="btn-group" role="group" aria-label="">
+            <button type="button" class="btn btn-default btn-toggle-breakpoint" title="{{ _('Toggle breakpoint') }}">
+                <i class="fa fa-circle"></i>
+            </button>
+            <button type="button" class="btn btn-default btn-clear-breakpoint" title="{{ _('Clear all breakpoints') }}">
+                <i class="fa fa-ban"></i>
+            </button>
+        </div>
+        <div class="btn-group" role="group" aria-label="">
+            <button type="button" class="btn btn-default btn-stop" title="{{ _('Stop') }}">
+                <i class="fa fa-stop-circle"></i>
+            </button>
         </div>
-      </ul>
     </div>
-  </div>
 </nav>
-<div id="container"></div>
+<div id="container" class="debugger-container"></div>
 {% endblock %}
 
 
diff --git a/web/pgadmin/tools/debugger/templates/debugger/js/direct.js b/web/pgadmin/tools/debugger/templates/debugger/js/direct.js
index 04fab8c..d7f2d78 100644
--- a/web/pgadmin/tools/debugger/templates/debugger/js/direct.js
+++ b/web/pgadmin/tools/debugger/templates/debugger/js/direct.js
@@ -1317,35 +1317,6 @@ define(
         'code', false, '100%', '100%',
         function(panel) {
 
-            var container = panel.layout().scene().find('.pg-debugger-panel');
-
-            // Create the wcSplitter used by wcDocker to split the single panel.
-            var hSplitter = new wcSplitter(
-                container, panel,
-                wcDocker.ORIENTATION.VERTICAL
-                );
-
-            hSplitter.scrollable(0, false, false);
-            hSplitter.scrollable(1, true, true);
-
-            // Initialize this splitter with a layout in each pane.
-            hSplitter.initLayouts(wcDocker.LAYOUT.SIMPLE, wcDocker.LAYOUT.SIMPLE);
-
-            // By default, the splitter splits down the middle, we split the main panel by 80%.
-            hSplitter.pos(0.65);
-
-            var params = $('<div class="full-container params"></div>');
-            hSplitter.right().addItem(params);
-
-            // Create wcDocker for tab set.
-            var out_docker = new wcDocker(
-              '.full-container', {
-              allowContextMenu: false,
-              allowCollapse: false,
-              themePath: '{{ url_for('static', filename='css/wcDocker/Themes') }}',
-              theme: 'pgadmin'
-            });
-
             // Create the parameters panel to display the arguments of the functions
             var parameters = new pgAdmin.Browser.Panel({
               name: 'parameters',
@@ -1402,49 +1373,50 @@ define(
             })
 
             // Load all the created panels
-            parameters.load(out_docker);
-            local_variables.load(out_docker);
-            messages.load(out_docker);
-            results.load(out_docker);
-            stack_pane.load(out_docker);
-
-            // Add all the panels to the docker
-            self.parameters_panel = out_docker.addPanel('parameters', wcDocker.DOCK.LEFT);
-            self.local_variables_panel = out_docker.addPanel('local_variables', wcDocker.DOCK.STACKED, self.parameters_panel);
-            self.stack_pane_panel = out_docker.addPanel('stack_pane', wcDocker.DOCK.STACKED, self.parameters_panel);
-            self.messages_panel = out_docker.addPanel('messages', wcDocker.DOCK.STACKED, self.parameters_panel);
-            self.results_panel = out_docker.addPanel('results', wcDocker.DOCK.STACKED, self.parameters_panel);
-
-            // Now create a tab widget and put that into one of the sub splits.
-            var editor_pane = $('<div id="stack_editor_pane" class="full-container-pane info"></div>');
-            var code_editor_area = $('<textarea id="debugger-editor-textarea"></textarea>').append(editor_pane);
-            hSplitter.left().addItem(code_editor_area);
-
-            // To show the line-number and set breakpoint marker details by user.
-            var editor = self.editor = CodeMirror.fromTextArea(
-                code_editor_area.get(0), {
-                lineNumbers: true,
-                gutters: ["note-gutter", "CodeMirror-linenumbers", "breakpoints"],
-                mode: "text/x-sql",
-                readOnly: true
-              });
+            parameters.load(self.docker);
+            local_variables.load(self.docker);
+            messages.load(self.docker);
+            results.load(self.docker);
+            stack_pane.load(self.docker);
+        });
+
+        self.code_editor_panel = self.docker.addPanel('code', wcDocker.DOCK.TOP );
+
+        self.parameters_panel = self.docker.addPanel(
+          'parameters', wcDocker.DOCK.BOTTOM, self.code_editor_panel);
+        self.local_variables_panel = self.docker.addPanel('local_variables', wcDocker.DOCK.STACKED, self.parameters_panel, {
+          tabOrientation: wcDocker.TAB.TOP
+        });
+        self.messages_panel = self.docker.addPanel('messages', wcDocker.DOCK.STACKED, self.parameters_panel);
+        self.results_panel = self.docker.addPanel(
+          'results', wcDocker.DOCK.STACKED, self.parameters_panel);
+        self.stack_pane_panel = self.docker.addPanel(
+          'stack_pane', wcDocker.DOCK.STACKED, self.parameters_panel);
+
+        var editor_pane = $('<div id="stack_editor_pane" class="full-container-pane info"></div>');
+        var code_editor_area = $('<textarea id="debugger-editor-textarea"></textarea>').append(editor_pane);
+        self.code_editor_panel.layout().addItem(code_editor_area);
+
+        // To show the line-number and set breakpoint marker details by user.
+        var editor = self.editor = CodeMirror.fromTextArea(
+          code_editor_area.get(0), {
+          lineNumbers: true,
+          gutters: ["note-gutter", "CodeMirror-linenumbers", "breakpoints"],
+          mode: "text/x-sql",
+          readOnly: true
         });
 
         // On loading the docker, register the callbacks
         var onLoad = function() {
-            self.docker.finishLoading(100);
-            self.docker.off(wcDocker.EVENT.LOADED);
-            // Register the callback when user set/clear the breakpoint on gutter area.
-            self.editor.on("gutterClick", self.onBreakPoint.bind(self), self);
+          self.docker.finishLoading(100);
+          self.docker.off(wcDocker.EVENT.LOADED);
+          // Register the callback when user set/clear the breakpoint on gutter area.
+          self.editor.on("gutterClick", self.onBreakPoint.bind(self), self);
         };
 
         self.docker.startLoading('{{ _('Loading...') }}');
         self.docker.on(wcDocker.EVENT.LOADED, onLoad);
 
-        self.main_panel = self.docker.addPanel(
-          'code', wcDocker.DOCK.TOP, null, {h: '100%', w: '100%'}
-          );
-
         // Create the toolbar view for debugging the function
         this.toolbarView = new DebuggerToolbarView();
     },


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: <CACCA4P26NCZhfVVn=AfM3sd1t-JvC-hzEzK84M-O8K6Vz-Jp-A@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