jonbrett / cinnamon-feeds-applet

Cinnamon applet for fetching and displaying RSS feeds
https://jonbrettdev.wordpress.com
20 stars 10 forks source link

Switch to external python app to get feeds #51

Closed collinss closed 9 years ago

collinss commented 9 years ago

Fixes #48

Please note that this fix currently only works on the development branch of Cinnamon. It wont work with the latest stable version due to it using the Util.spawn_async function.

I haven't tested this rigorously, so there may be bugs, but it was able to handle several rss feeds, and an atom feed just fine.

mockturtl commented 9 years ago

:+1: I can verify this change gets the feed reader working again.

Required changes against the Cinnamon 2.4.8 stable release follow. (see linuxmint/Cinnamon@a1639083e2ae2f55cc25d4534216145ef3f11feb)

(@collinss Is this formatted correctly? I haven't used diff/patch much.)

--- /usr/share/cinnamon/js/ui/cinnamonDBus.js   2015-05-01 15:21:55.593719033 -0400
+++ /usr/share/cinnamon/js/ui/cinnamonDBus.js   2015-05-01 15:17:40.296227228 -0400
@@ -9,6 +9,8 @@
 const AppletManager = imports.ui.appletManager;
 const DeskletManager = imports.ui.deskletManager;
 const ExtensionSystem = imports.ui.extensionSystem;
+const Util = imports.misc.util;

 const CinnamonIface =
     '<node> \
@@ -76,6 +78,10 @@
                 <arg type="b" direction="out" /> \
                 <arg type="s" direction="out" /> \
             </signal> \
+            <method name="PushSubprocessResult"> \
+                <arg type="i" direction="in" name="process_id" /> \
+                <arg type="s" direction="in" name="result" /> \
+            </method> \
         </interface> \
     </node>';

@@ -307,6 +313,14 @@
         if (!Main.expo.animationInProgress)
             Main.expo.toggle();
     },
+    
+    PushSubprocessResult: function(process_id, result)
+    {
+        if (Util.subprocess_callbacks[process_id])
+        {
+            Util.subprocess_callbacks[process_id](result);
+        }
+    },

     CinnamonVersion: Config.PACKAGE_VERSION
 };
\ No newline at end of file
--- -   2015-05-01 15:23:45.844786156 -0400
+++ /usr/bin/cinnamon-subprocess-wrapper    2015-05-01 15:19:06.332685814 -0400
@@ -0,0 +1,14 @@
+#! /usr/bin/env python2
+# -*- coding=utf-8 -*-
+
+import subprocess
+import sys
+import dbus
+
+if __name__ == "__main__":
+    process_id = int(sys.argv[1])
+    result = subprocess.check_output(sys.argv[2:])
+    session_bus = dbus.SessionBus()
+    dbus = session_bus.get_object("org.Cinnamon", "/org/Cinnamon")
+    PushSubprocessResult = dbus.get_dbus_method('PushSubprocessResult', 'org.Cinnamon')
+    PushSubprocessResult(process_id, result)
\ No newline at end of file
--- /usr/share/cinnamon/js/misc/util.js 2015-05-01 15:21:12.927475139 -0400
+++ /usr/share/cinnamon/js/misc/util.js 2015-05-01 15:18:22.814477094 -0400
@@ -63,6 +63,14 @@
     return pid;
 }

+let subprocess_id = 0;
+let subprocess_callbacks = {};
+function spawn_async(args, callback) {
+    subprocess_id++;
+    subprocess_callbacks[subprocess_id] = callback;
+    spawn(new Array("cinnamon-subprocess-wrapper", subprocess_id.toString()).concat(args));
+}
+
 // spawnCommandLine:
 // @command_line: a command line
 //
collinss commented 9 years ago

@mockturtl looks right to me, but I'm not an expert myself. I usually let git or github handle the diffs. As a side-note, there was an update in the last couple of days that allows multi-version support for all extensions. All you have to do is add "multiversion": true to metadata.json and put the version of the applet/desklet/extension in <uuid>/min_version/, where min_version is the minimum version of Cinnamon that is supported by that version of the applet/desklet/extension.

mockturtl commented 9 years ago

As a side-note, there was an update in the last couple of days that allows multi-version support for all extensions.

Neat, thanks.

Incidentally, I'm stumped on how to get rid of this "Loading feed" block:

submenu

I see where the label text is set, but I can't figure out what (actor|box|child) to (remove|clear) so that the feed title will left-align correctly. Is it in a column, or a separate table...?

collinss commented 9 years ago

I have no idea. I'm not really familiar with most of the code in the applet. I'd have to scour the code to find out, and I've got a lot going on right now.

mockturtl commented 9 years ago

Understood. @jonbrett?

jonbrett commented 9 years ago

@mockturtl Hmm... the applet relied on digging around inside PopupMenu.PopupSubMenuMenuItem to remove this messae. Clearly this class has changed since Cinnamon 2.0, where it was last working. You will need to dig around inside PopupMenu.PopupSubMenuMenuItem to figure out how to remove the label added during the initialization.

mockturtl commented 9 years ago

I've verified the original PR makes the applet work in 2.6.2, with one adjustment:

Four appearances of s.icon.icon_type = St.IconType.SYMBOLIC; need removing. Applet.MenuItem is deprecated, and redirects to a different type that does not expose icon.

(details 1, 2)

mockturtl commented 9 years ago

@jonbrett Any chance of getting this merged and released?

jonbrett commented 9 years ago

Sorry, for not being active enough again... I will merge and release today

jonbrett commented 9 years ago

I have included this in a new 0.13 release and posted it onto spices: http://cinnamon-spices.linuxmint.com/applets/view/149

jonbrett commented 9 years ago

Thanks again, @collinss and @mockturtl for following-up on this.

mockturtl commented 9 years ago

@jonbrett Thanks! It will need this, though --

Four appearances of s.icon.icon_type = St.IconType.SYMBOLIC; need removing. Applet.MenuItem is deprecated, and redirects to a different type that does not expose icon. (details 1, 2)

See #52.