Merge lp:~mefrio-g/scratch/fix-1258260 into lp:~elementary-apps/scratch/scratch

[Mario Guerriero]

Don't show outline pane if there is nothing to outline. Fixes bug #1258260

[Niclas Lockner (niclasl)]

The filter_generated_fields method in ValaSymbolResolver in unused now that the call at diff line 118 is removed. Should n't it be removed as well?

I noticed three situations where it doesn't work as expected:

When I start Scratch, the empty outline pane is shown alongside the welcome screen.

If the outline pane is shown when the last tab is closed (e.g. one single vala file opened and then closed), the outline pane is still shown alongside the welcome screen. If I instead let the last closed tab be a non-vala file, it works as expected.

If I open some new tabs with new or existing non-vala documents and then switch between them, the outline pane appears. It hides itself when I open yet another tab.

review: Needs Fixing

[Niclas Lockner (niclasl)]

And when I say that the pane is shown alongside the welcome screen, I mean that the buttons on the welcome screen are no longer centered and there's a large empty space to the right where the pane will be once a vala file is opened.

[lp:~mefrio-g/scratch/fix-1258260]

1278. By Mario Guerriero on 2014-04-23

fixed welcome screen and unsaved documents problems

[Mario Guerriero (mefrio-g)]

It should work as expected now. I also fixed some compilation warnings.

[Niclas Lockner (niclasl)]

It still doesn't work that well for me:

Open scratch (No tabs are open and the welcome screen is shown)

Open a Vala file
=> Outline pane has two Symbols tabs (the inactive tab isn't clickable)

Open a second Vala file
Switch between the tabs
=> The output pane no longer shows any content
=> For each switch, a new Symbols tab is added to the Outline pane

Close both tabs
=> failed assertion and SIGABRT when the last tab is closed

If I instead open Scratch, create a new document and then open a Vala document, it seems to work.
I can switch between the two tabs and the outline pane is shown or hidden accordingly. The outline pane also has the right content.

review: Needs Fixing

[lp:~mefrio-g/scratch/fix-1258260]

1279. By Mario Guerriero on 2014-04-23

do not add several useless symbols view to the left pane

[Mario Guerriero (mefrio-g)]

Everything should work as expected now. Thanks for your patience.

[Niclas Lockner (niclasl)]

Now it works much better, but there's still one bug:

If I open a vala file and a regular file (not a new document but an existing one) and then switch tab from the vala file to the non-vala file, the outline pane becomes empty but doesn't hide as it should.

If I instead open a vala file and a new empty document, it works as long as the new document isn't saved. Once saved, Scratch behaves as above.

Also, there's a conflict with trunk's ValaSymbolResolver.vala

review: Needs Fixing

[Niclas Lockner (niclasl)]

And if the code style is to be strictly followed:

Newline before diff line 26
Remove spaces on diff lines 66, 97, 104, 137, 139
Add space after if on diff line 99
Make a cuddled else if on diff lines 92-93

[lp:~mefrio-g/scratch/fix-1258260]

1280. By Mario Guerriero on 2014-04-23

fixed switch between vala and non-vala files

[Mario Guerriero (mefrio-g)]

It now works fine. I see the conflict project but to be honest I don't know how to fix it. Could you help me?

[Niclas Lockner (niclasl)]

Yes, now it works as expected :) Just the code style issues and the conflict left then.

To resolve the conflict, first merge with trunk:
bzr merge lp:scratch

which should give you:
Text conflict in plugins/outline/ValaSymbolResolver.vala
1 conflicts encountered.

After that, open plugins/outline/ValaSymbolResolver.vala and edit it so that it looks good (i.e. remove line 167-183 in this case)

After that, mark all conflicts as resolved:
bzr resolve

and commit the changes:
bzr commit -m "Merge changes from lp:scratch"

[lp:~mefrio-g/scratch/fix-1258260]

1281. By Mario Guerriero on 2014-04-25

Merge changes from lp:scratch

[Mario Guerriero (mefrio-g)]

It should be everything fine now. Thanks.

[Niclas Lockner (niclasl)]

review: Approve

=== modified file 'plugins/outline/CtagsSymbolResolver.vala'
--- plugins/outline/CtagsSymbolResolver.vala	2014-04-13 08:35:48 +0000
+++ plugins/outline/CtagsSymbolResolver.vala	2014-04-25 14:53:01 +0000
@@ -30,6 +30,8 @@
     Granite.Widgets.SourceList store;
     Granite.Widgets.SourceList.ExpandableItem root;
 
+    public int n_symbols { get; protected set; }
+
     public CtagsSymbolOutline (Scratch.Services.Document _doc)
     {
         doc = _doc;
@@ -37,6 +39,8 @@
 
         root = new Granite.Widgets.SourceList.ExpandableItem (_("Symbols"));
 
+        this.n_symbols = 0;
+
         store = new Granite.Widgets.SourceList ();
         store.get_style_context ().add_class ("sidebar");
         store.item_selected.connect ((selected) => {
@@ -133,6 +137,7 @@
             } else
                 parent_dependent.add (new CtagsSymbolIter (name, parent, line, icon));
         }
+        this.n_symbols++;
 
         var found_something = true;
         while (found_something && parent_dependent.size > 0) {
@@ -173,6 +178,8 @@
     void parse_fields (string fields, out int line, out string parent)
     {
         var index = -1;
+        line = -1;
+        parent = null;
         if ((index = fields.index_of ("line:")) > -1) {
             line = int.parse (fields.substring (index + 5, int.max (fields.index_of (" ", index + 6) - index, -1)));
         }

=== modified file 'plugins/outline/OutlinePlugin.vala'
--- plugins/outline/OutlinePlugin.vala	2014-04-13 08:35:48 +0000
+++ plugins/outline/OutlinePlugin.vala	2014-04-25 14:53:01 +0000
@@ -25,6 +25,7 @@
 {
     public abstract Scratch.Services.Document doc { get; protected set; }
     public abstract void parse_symbols ();
+    public abstract int n_symbols { get; protected set; }
     public abstract Granite.Widgets.SourceList get_source_list ();
     public signal void closed ();
     public signal void goto (Scratch.Services.Document doc, int line);
@@ -37,6 +38,7 @@
         Scratch.Services.Interface scratch_interface;
         SymbolOutline? current_view = null;
         Gtk.EventBox? container = null;
+        Gtk.Notebook? notebook = null;
 
         uint refresh_timeout = 0;
 
@@ -60,11 +62,11 @@
         void on_hook_context (Gtk.Notebook notebook) {
             if (container != null)
                 return;
-            
+            if (this.notebook == null)
+                this.notebook = notebook;
+                
             container = new Gtk.EventBox ();
-            container.visible = false;            
-            notebook.append_page (container, new Gtk.Label (_("Symbols")));
-            container.show_all ();
+            container.visible = false;
         }
 
         void on_hook_document (Scratch.Services.Document doc) {
@@ -81,7 +83,7 @@
                     break;
                 }
             }
-            if (view == null) {
+            if (view == null && doc.file != null) {
                 if (doc.get_mime_type () == "text/x-vala") {
                     view = new ValaSymbolOutline (doc);
                 } else {
@@ -98,14 +100,33 @@
             container.add (view.get_source_list ());
             container.show_all ();
             current_view = view;
+
+            if (view.n_symbols > 1) {
+                add_container ();
+            }
+            else if (doc.file == null || view.n_symbols <= 1) {
+                remove_container ();
+            }
+        }
+        
+        void add_container () {
+            if(notebook.page_num (container) == -1) {
+                notebook.append_page (container, new Gtk.Label (_("Symbols")));
+                container.show_all ();
+            }
+        }
+        
+        void remove_container () {
+            if (notebook.page_num (container) != -1)
+                notebook.remove (container);
         }
         
         void on_hook_split_view (Scratch.Widgets.SplitView view) {
             view.welcome_shown.connect (() => {
-                container.visible = false;
+                remove_container ();
             });
             view.welcome_hidden.connect (() => {
-                container.visible = true;
+                add_container ();
             });
         }
         
@@ -144,4 +165,4 @@
 {
   var objmodule = module as Peas.ObjectModule;
   objmodule.register_extension_type (typeof (Peas.Activatable), typeof (Scratch.Plugins.OutlinePlugin));
-}
+}
\ No newline at end of file

=== modified file 'plugins/outline/ValaSymbolResolver.vala'
--- plugins/outline/ValaSymbolResolver.vala	2014-04-23 11:16:47 +0000
+++ plugins/outline/ValaSymbolResolver.vala	2014-04-25 14:53:01 +0000
@@ -40,7 +40,9 @@
     Vala.CodeContext context;
     Vala.Parser parser;
     SymbolResolver resolver;
-
+    
+    public int n_symbols { get; protected set; }
+    
     SymbolIter cache;
 
     Gee.List<Vala.Field> field_blacklist;
@@ -70,6 +72,8 @@
             field_blacklist.add (f);
         });
 
+        this.n_symbols = 0;
+
         init_context ();
     }
 
@@ -117,7 +121,7 @@
         init_context ();
         
         Thread<void*> thread = new Thread<void*>("parse-symbols", () => {
-            parse_symbols_async ();
+            parse_symbols_async.begin ();
             return null;
         });
 
@@ -126,8 +130,6 @@
         root.clear ();
         construct_tree (cache, root);
 
-        filter_generated_fields (root);
-
         store.root.expand_all ();
     }
 
@@ -135,11 +137,14 @@
         Granite.Widgets.SourceList.ExpandableItem tree_parent)
     {
         foreach (var iter_child in iter_parent.children) {
+            if (iter_child == null)
+                continue;
             var tree_child = new Symbol (doc, iter_child.symbol);
             tree_child.icon = iter_child.icon;
             tree_parent.add (tree_child);
 
             construct_tree (iter_child, tree_child);
+            this.n_symbols++;
         }
     }
 
@@ -159,20 +164,6 @@
         return match;
     }
 
-    // vala generates for each property a field which we do not want to display
-    void filter_generated_fields (Granite.Widgets.SourceList.ExpandableItem parent)
-    {
-        var children = parent.children.to_array ();
-        foreach (var child in children) {
-            var child_symbol = child as Symbol;
-            if (field_blacklist.contains (child_symbol.symbol as Vala.Field)) {
-                parent.remove (child);
-            } else {
-                filter_generated_fields (child_symbol);
-            }
-        }
-    }
-
     void add_symbol (Vala.Symbol symbol, string icon = "", Icon? real_icon = null)
     {
         if (symbol.name == null)