Merge lp:~mhr3/unity-lens-applications/fix-842108 into lp:unity-lens-applications

[Michal Hruby]

Don't remove exact matches from list of suggestions

"Run command" lens doesn't include exact match when there are also partial matches.

Don't remove the exact match from list of suggestions and split out the searcher into a separate class to make it easier to unit test in the future.

Added unity-tool tests.

[Didier Roche-Tolomelli (didrocks)]

LGTM, thanks Michal!

review: Approve

=== modified file 'src/runner.vala'
--- src/runner.vala	2012-02-22 15:14:36 +0000
+++ src/runner.vala	2012-04-26 07:07:18 +0000
@@ -50,8 +50,8 @@
     public Unity.Scope scope;
 
     private Gee.HashMap<string,AboutEntry> about_entries;
-    private Gee.List<string> executables;
     private Gee.List<string> history;
+    private ExecSearcher exec_searcher = new ExecSearcher ();
 
     private Settings gp_settings;
     
@@ -73,12 +73,6 @@
         return lens_search.search_string.strip ();
       });
 
-      /* Re-do the search if the filters changed */
-      scope.filters_changed.connect (() =>
-      {
-        scope.queue_search_changed (SearchType.DEFAULT);
-      });
-
       // Listen for when the lens is hidden/shown by the Unity Shell
       scope.notify["active"].connect(() =>
       {
@@ -104,9 +98,6 @@
       about_entries = new Gee.HashMap<string,AboutEntry> ();
       load_about_entries ();
       
-      executables = new Gee.ArrayList<string> ();
-      find_system_executables.begin ();
-
       this.gp_settings = new Settings ("com.canonical.Unity.Runner");
       history = new Gee.ArrayList<string> ();
       load_history ();
@@ -219,7 +210,7 @@
               break;
             
             var complete_path = Path.build_filename (search_dirname, subelem_info.get_name ());
-            if (complete_path.has_prefix (search_string) && complete_path != search_string)
+            if (complete_path.has_prefix (search_string))
             {
               if (subelem_info.get_file_type () == FileType.DIRECTORY)
               {
@@ -244,12 +235,10 @@
       /* complete again system executables */
       else
       {
-        foreach (var exec_candidate in this.executables)
+        var matching_executables = yield exec_searcher.find_prefixed (search_string);
+        foreach (var matching_exec in matching_executables)
         {
-          if (exec_candidate.has_prefix (search_string) && exec_candidate != search_string)
-          {
-            executables_match.add (exec_candidate);
-          }
+          executables_match.add (matching_exec);
         }
       }
       
@@ -302,35 +291,91 @@
       search.finished ();
     }
     
-    // TODO: add reload
-    private async void find_system_executables ()
+    private class ExecSearcher: Object
     {
-
-      if (this.executables.size > 0)
-        return;
-       
-      foreach (var path_directory in Environment.get_variable ("PATH").split(":"))
-      {
-        var dir = File.new_for_path (path_directory);
-        try {
-          var e = yield dir.enumerate_children_async (FILE_ATTRIBUTE_STANDARD_NAME + "," + FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE,
-                                                      0, Priority.DEFAULT, null);
-          while (true) {
-            var files = yield e.next_files_async (10, Priority.DEFAULT, null);
-            if (files == null)
-                break;
-
-            foreach (var info in files) {
-              if (info.get_attribute_boolean (FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE))
-              {
-                this.executables.add (info.get_name ());
+      public ExecSearcher ()
+      {
+        Object ();
+      }
+
+      private Gee.List<string> executables;
+
+      construct
+      {
+        executables = new Gee.ArrayList<string> ();
+        listing_status = ListingStatus.NOT_STARTED;
+      }
+
+      // TODO: add reload
+      private async void find_system_executables ()
+      {
+        if (this.executables.size > 0)
+          return;
+
+        foreach (var path_directory in Environment.get_variable ("PATH").split(":"))
+        {
+          var dir = File.new_for_path (path_directory);
+          try {
+            var e = yield dir.enumerate_children_async (FILE_ATTRIBUTE_STANDARD_NAME + "," + FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE,
+                                                        0, Priority.DEFAULT, null);
+            while (true) {
+              var files = yield e.next_files_async (64, Priority.DEFAULT, null);
+              if (files == null)
+                  break;
+
+              foreach (var info in files) {
+                if (info.get_attribute_boolean (FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE))
+                {
+                  this.executables.add (info.get_name ());
+                }
               }
             }
           }
-        }
-        catch (Error err) {
-          warning("Error listing directory executables: %s\n", err.message);
-        }
+          catch (Error err) {
+            warning("Error listing directory executables: %s\n", err.message);
+          }
+        }
+      }
+
+      private enum ListingStatus
+      {
+        NOT_STARTED,
+        STARTED,
+        FINISHED
+      }
+
+      public int listing_status { get; private set; }
+
+      public async Gee.Collection<string> find_prefixed (string search_string)
+      {
+        // initialize the available binaries lazily
+        if (listing_status == ListingStatus.NOT_STARTED)
+        {
+          listing_status = ListingStatus.STARTED;
+          yield find_system_executables ();
+          listing_status = ListingStatus.FINISHED;
+        }
+        else if (listing_status == ListingStatus.STARTED)
+        {
+          var sig_id = this.notify["listing-status"].connect (() =>
+          {
+            if (listing_status == ListingStatus.FINISHED)
+              find_prefixed.callback ();
+          });
+          yield;
+          SignalHandler.disconnect (this, sig_id);
+        }
+
+        var matching = new Gee.ArrayList<string> ();
+        foreach (var exec_candidate in executables)
+        {
+          if (exec_candidate.has_prefix (search_string))
+          {
+            matching.add (exec_candidate);
+          }
+        }
+
+        return matching;
       }
     }
     
@@ -358,7 +403,7 @@
         icon = ContentType.get_icon (mimetype);
         return exec_string;
       }
-                        
+
       var s = exec_string.delimit ("-", '_').split (" ", 0)[0];      
       var appresults = this.daemon.appsearcher.search (@"type:Application AND exec:$s", 0,
                                                        Unity.Package.SearchType.EXACT,

=== added file 'tests/test-full-and-partial.sh'
--- tests/test-full-and-partial.sh	1970-01-01 00:00:00 +0000
+++ tests/test-full-and-partial.sh	2012-04-26 07:07:18 +0000
@@ -0,0 +1,7 @@
+#!/bin/sh
+
+# Search for "ls", print the results, this search should only include both
+# "ls", not "ls.+"
+test `unity-tool -n com.canonical.Unity.Lens.Applications -p /com/canonical/unity/lens/commands -s "ls" --no-search-reply -r | grep "\\<ls\\>" | wc -l` -eq 1 || exit 1
+test `unity-tool -n com.canonical.Unity.Lens.Applications -p /com/canonical/unity/lens/commands -s "ls" --no-search-reply -r | grep "\\<ls" | wc -l` -ge 2 || exit 1
+

=== added file 'tests/test-no-partial-commands.sh'
--- tests/test-no-partial-commands.sh	1970-01-01 00:00:00 +0000
+++ tests/test-no-partial-commands.sh	2012-04-26 07:07:18 +0000
@@ -0,0 +1,6 @@
+#!/bin/sh
+
+# Search for "modpro", print the results, this search should only include
+# "modprobe", not "modpro" itself
+test `unity-tool -n com.canonical.Unity.Lens.Applications -p /com/canonical/unity/lens/commands -s "modpro" --no-search-reply -r | grep "modpro" | wc -l` -eq `unity-tool -n com.canonical.Unity.Lens.Applications -p /com/canonical/unity/lens/commands -s "modpro" --no-search-reply -r | grep "modprobe" | wc -l` || exit 1
+