Merge lp:~3v1n0/bamf/class-to-index-file into lp:bamf/0.4

[Marco Trevisan (Treviño)]

debian/bamfdaemon.postinst: write it in multiline and add support for StartupWMClass

And other properties such as NoDisplay and OnlyShowIn.

Also rewritten the bamfdaemon.postinst to use a multi-line perl script for easier review.

Save to the /usr/share/applications/bamf.index columns containing extra parameters such as StartupWMClass, NoDisplay and OnlyShowIn. Update the bamf-matcher to use them instead of parse these from .desktop files, saving some IO.

[lp:~3v1n0/bamf/class-to-index-file]

527. By Marco Trevisan (Treviño) on 2013-03-28

debian/bamfdaemon.postinst: indentation fix

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:527
http://jenkins.qa.ubuntu.com/job/bamf-ci/1/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/bamf-raring-amd64-ci/1/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/bamf-raring-armhf-ci/1/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/bamf-ci/1/rebuild

review: Needs Fixing (continuous-integration)

[Loïc Minier (lool)]

Hey,

r525: ok; the bits of Perl are written like C, but eh decently readable and it seems like they will work :-)

r526:
- strstr() seems a bit weak to match OnlyShowIn; that would work for now, but what if we want to implement e.g. Unity-Next in .desktop files? might be best to split on ";" properly here
- is it ok to g_hash_table_insert direclty instead of using something like insert_desktop_file_class_into_table?

r527: ok

Some problems not directly related, but worth fixing:
* no version information in bamf index; you could have a version depending on the filename (e.g. this could be bamf-2.0.index)
* should this be in /var/cache or /var/lib, but not in /usr/share? it's actually only modified when packages are added/removed (.desktop files), but seems a bit weird to have a locally generated file under /usr/share
* generally, I haven't checked how whitespace is handled in .desktop files (e.g. if the .desktop file contains trailing or leading spaces or tabs around key names / key values)

(not sure how ~/ .desktop are handled, but I guess that's another story)

Thanks,

[Marco Trevisan (Treviño) (3v1n0)]

> Hey,
>
> r525: ok; the bits of Perl are written like C, but eh decently readable and it
> seems like they will work :-)

We needed to clean them up :)

> r526:
> - strstr() seems a bit weak to match OnlyShowIn; that would work for now, but
> what if we want to implement e.g. Unity-Next in .desktop files? might be best
> to split on ";" properly here

Ok, it was my same idea at the beginning, then I forgot to finish it waiting to test this :)

> - is it ok to g_hash_table_insert direclty instead of using something like
> insert_desktop_file_class_into_table?

Yes. Using insert_desktop_file_class_into_table causes to open the file again, and so to use io-blocking functions, while we want to avoid this since this data is already provided by the index file.

> r527: ok
>
> Some problems not directly related, but worth fixing:
> * no version information in bamf index; you could have a version depending on
> the filename (e.g. this could be bamf-2.0.index)

Yes, it makes sense... Even if this file will be always built before the new bamf will able to use it so it should be quite safe.

> * should this be in /var/cache or /var/lib, but not in /usr/share? it's
> actually only modified when packages are added/removed (.desktop files), but
> seems a bit weird to have a locally generated file under /usr/share

Yes, probably true. But the idea is that you can also add a bamf.index file inside another monitored folder and use it instead of manual directory parsing.

> * generally, I haven't checked how whitespace is handled in .desktop files
> (e.g. if the .desktop file contains trailing or leading spaces or tabs around
> key names / key values)

There shouldn't be issues for this. Also luckily there are no problems with the provided .desktop files in raring, but maybe having stronger regex could help.

[Brandon Schaefer (brandontschaefer)]

LGTM. Tests pass as well.

review: Approve

[lp:~3v1n0/bamf/class-to-index-file]

528. By Marco Trevisan (Treviño) on 2013-03-28

debian/bamfdaemon.postinst: trim the parsed strings

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:528
http://jenkins.qa.ubuntu.com/job/bamf-ci/5/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/bamf-raring-amd64-ci/5/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/bamf-raring-armhf-ci/5/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/bamf-ci/5/rebuild

review: Needs Fixing (continuous-integration)

[lp:~3v1n0/bamf/class-to-index-file]

529. By Marco Trevisan (Treviño) on 2013-03-28

Use g_strsplit to handle OnlyShowIn values

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:529
http://jenkins.qa.ubuntu.com/job/bamf-ci/6/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/bamf-raring-amd64-ci/6/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/bamf-raring-armhf-ci/6/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/bamf-ci/6/rebuild

review: Needs Fixing (continuous-integration)

[lp:~3v1n0/bamf/class-to-index-file]

530. By Marco Trevisan (Treviño) on 2013-03-28

BamfMatcher: use bamf-2.index file

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:530
http://jenkins.qa.ubuntu.com/job/bamf-ci/7/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/bamf-raring-amd64-ci/7/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/bamf-raring-armhf-ci/7/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/bamf-ci/7/rebuild

review: Needs Fixing (continuous-integration)

[Loïc Minier (lool)]

Looks good; thanks for the fixes, now why does it break jenkins?
cc: error: unrecognized command line option '--c-include=libbamf/libbamf.h'

review: Approve

[Marco Trevisan (Treviño) (3v1n0)]

> Looks good; thanks for the fixes, now why does it break jenkins?
> cc: error: unrecognized command line option '--c-include=libbamf/libbamf.h'

There have been changes to glib and the gobject introspection not related to this branch, the fixes are landing right now in lp:~3v1n0/bamf/fix-trunk-compilation-tests

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'debian/bamfdaemon.postinst'
--- debian/bamfdaemon.postinst	2012-10-17 06:02:25 +0000
+++ debian/bamfdaemon.postinst	2013-03-28 20:01:22 +0000
@@ -1,13 +1,84 @@
 #! /bin/sh
 set -e
 
+BAMF_INDEX_NAME="bamf-2.index"
+
 if [ "$1" = "configure" ] || [ "$1" = "triggered" ]; then
     if [ -d /usr/share/applications ]; then
         # rebuild index
-        I=/usr/share/applications/bamf.index
-	echo "Rebuilding $I..."
+        I=/usr/share/applications/$BAMF_INDEX_NAME
+        echo "Rebuilding $I..."
         rm -f $I
-        perl -e 'use File::Find; my $dir_name; sub strip_exec { my $f = $_; return unless $f =~ /\.desktop$/; return unless ("$File::Find::dir" eq "$dir_name"); my @lines; open F, $f; @lines = <F>; close F; my $in_desktop_entry = 0; foreach (@lines) { if (/^\[\s*(.+)\s*\]/) { $in_desktop_entry = ($1 eq "Desktop Entry"); } if (/^Exec=(.+)$/ && $in_desktop_entry) { print "$f\t$1\n"; } } }; $dir_name = $ARGV[-1]; $dir_name = $1 if($dir_name=~/(.*)\/$/); find(\&strip_exec, $dir_name);' /usr/share/applications > $I.new || rm -f $I.new
+        perl -e '
+use File::Find;
+my $dir_name;
+
+sub strip_exec
+{
+    my $f = $_;
+    return unless $f =~ /\.desktop$/;
+    return unless ("$File::Find::dir" eq "$dir_name");
+    my @lines;
+    open F, $f;
+    @lines = <F>;
+    close F;
+    my $in_desktop_entry = 0;
+    my $exec = "";
+    my $class = "";
+    my $no_display = "false";
+    my $only_show_in = "";
+
+    foreach (@lines)
+    {
+        if (/^\[\s*(.+)\s*\]/)
+        {
+            $was_in_desktop = $in_desktop_entry;
+            $in_desktop_entry = ($1 eq "Desktop Entry");
+
+            if ($was_in_desktop and !$in_desktop_entry)
+            {
+                last;
+            }
+        }
+
+        if ($in_desktop_entry)
+        {
+            if (/^Exec=(.+)$/)
+            {
+                $exec = $1;
+                $exec =~ s/^\s+|\s+$//g;
+            }
+
+            if (/^NoDisplay=(.+)$/)
+            {
+                $no_display = $1;
+                $no_display =~ s/^\s+|\s+$//g;
+            }
+
+            if (/^OnlyShowIn=(.+)$/)
+            {
+                $only_show_in = $1;
+                $only_show_in =~ s/^\s+|\s+$//g;
+            }
+
+            if (/^StartupWMClass=(.+)$/)
+            {
+                $class = $1;
+                $class =~ s/^\s+|\s+$//g;
+            }
+        }
+    }
+
+    if ($exec ne "")
+    {
+        print "$f\t$exec\t$class\t$only_show_in\t$no_display\n";
+    }
+};
+
+$dir_name = $ARGV[-1];
+$dir_name = $1 if($dir_name=~/(.*)\/$/);
+find(\&strip_exec, $dir_name);' /usr/share/applications > $I.new || rm -f $I.new
+
         [ -e $I.new ] && mv $I.new $I || true
     fi
 fi
 
=== modified file 'src/bamf-matcher.c'
--- src/bamf-matcher.c	2013-03-06 04:29:49 +0000
+++ src/bamf-matcher.c	2013-03-28 20:01:22 +0000
@@ -31,6 +31,8 @@
 #endif
 #include <strings.h>
 
+#define BAMF_INDEX_NAME "bamf-2.index"
+
 G_DEFINE_TYPE (BamfMatcher, bamf_matcher, BAMF_DBUS_TYPE_MATCHER_SKELETON);
 #define BAMF_MATCHER_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE(obj, \
                                        BAMF_TYPE_MATCHER, BamfMatcherPrivate))
@@ -938,6 +940,7 @@
   GDataInputStream *input;
   char *line;
   char *directory;
+  const char *current_desktop;
   gsize length;
 
   file = g_file_new_for_path (index_file);
@@ -952,16 +955,51 @@
       return;
     }
 
+  length = 0;
+  current_desktop = g_getenv ("XDG_CURRENT_DESKTOP");
   directory = g_path_get_dirname (index_file);
   input = g_data_input_stream_new (G_INPUT_STREAM (stream));
 
+  if (current_desktop && current_desktop[0] == '\0')
+    current_desktop = NULL;
+
   while ((line = g_data_input_stream_read_line (input, &length, NULL, NULL)))
     {
       char *exec;
       char *filename;
+      const char *class;
+      const char *show_in;
       GString *desktop_id;
 
-      gchar **parts = g_strsplit (line, "\t", 3);
+      /* Order is: 0 Desktop-Id, 1 Exec, 2 class, 3 ShowIn, 4 NoDisplay */
+      gchar **parts = g_strsplit (line, "\t", 5);
+
+      show_in = parts[3];
+
+      if (current_desktop && show_in && show_in[0] != '\0')
+        {
+          gchar **sub_parts = g_strsplit (show_in, ";", -1);
+          gboolean found_current = FALSE;
+          int i = 0;
+
+          for (i = 0; sub_parts[i]; ++i)
+            {
+              if (g_ascii_strcasecmp (sub_parts[i], current_desktop) == 0)
+                {
+                  found_current = TRUE;
+                  break;
+                }
+            }
+
+          g_strfreev (sub_parts);
+
+          if (!found_current)
+            {
+              length = 0;
+              g_strfreev (parts);
+              continue;
+            }
+        }
 
       char *tmp = bamf_matcher_get_trimmed_exec (self, parts[1]);
       g_free (parts[1]);
@@ -974,7 +1012,12 @@
       g_string_truncate (desktop_id, desktop_id->len - 8);
 
       insert_data_into_tables (self, filename, exec, desktop_id->str, desktop_file_table, desktop_id_table);
-      insert_desktop_file_class_into_table (self, filename, desktop_class_table);
+
+      class = parts[2];
+      if (class && class[0] != '\0')
+        {
+          g_hash_table_insert (desktop_class_table, g_strdup (filename), g_strdup (class));
+        }
 
       g_string_free (desktop_id, TRUE);
       g_free (line);
@@ -1349,7 +1392,7 @@
 
       bamf_matcher_add_new_monitored_directory (self, directory);
 
-      bamf_file = g_build_filename (directory, "bamf.index", NULL);
+      bamf_file = g_build_filename (directory, BAMF_INDEX_NAME, NULL);
 
       if (g_file_test (bamf_file, G_FILE_TEST_EXISTS))
         {