Let scripts register actions at predefined locations in the UI

src/CMakeLists.txt

@@ -696,12 +696,14 @@ if(HAVE_SCRIPTING)
     scripting/scriptdialogtabwidget.cpp
     scripting/scriptinterface.cpp
     scripting/scriptmanager.cpp
+    scripting/uiinterface.cpp
   )
   list(APPEND HEADERS
     scripting/scriptdialog.h
     scripting/scriptdialogtabwidget.h
     scripting/scriptinterface.h
     scripting/scriptmanager.h
+    scripting/uiinterface.h
   )
   list(APPEND UI
     scripting/scriptdialog.ui

src/scripting/languageengine.h

@@ -34,6 +34,7 @@ public:
 
   virtual Script* CreateScript(const QString& path, const QString& script_file,
                                const QString& id) = 0;
+  virtual void DestroyScript(Script* script) = 0;
 
 private:
   ScriptManager* manager_;

src/scripting/python/clementine.sip

@@ -11,3 +11,4 @@
 %Include pythonengine.sip
 %Include scriptinterface.sip
 %Include song.sip
+%Include uiinterface.sip

src/scripting/python/pythonengine.cpp

@@ -16,6 +16,7 @@
 */
 
 #include <Python.h>
+#include <frameobject.h>
 #include <sip.h>
 
 #include "pythonengine.h"
@@ -23,8 +24,10 @@
 #include "sipAPIclementine.h"
 
 #include <QFile>
+#include <QtDebug>
 
 const char* PythonEngine::kModulePrefix = "clementinescripts";
+PythonEngine* PythonEngine::sInstance = NULL;
 
 extern "C" {
   void initclementine();
@@ -34,6 +37,12 @@ PythonEngine::PythonEngine(ScriptManager* manager)
   : LanguageEngine(manager),
     initialised_(false)
 {
+  Q_ASSERT(sInstance == NULL);
+  sInstance = this;
+}
+
+PythonEngine::~PythonEngine() {
+  sInstance = NULL;
 }
 
 const sipAPIDef* PythonEngine::GetSIPApi() {
@@ -98,6 +107,7 @@ Script* PythonEngine::CreateScript(const QString& path,
     // Add objects to the module
     AddObject(manager()->data().player_, sipType_Player, "player");
     AddObject(manager()->data().playlists_, sipType_PlaylistManager, "playlists");
+    AddObject(manager()->ui(), sipType_UIInterface, "ui");
     AddObject(this, sipType_PythonEngine, "pythonengine");
 
     // Create a module for scripts
@@ -121,15 +131,21 @@ Script* PythonEngine::CreateScript(const QString& path,
   }
 
   Script* ret = new PythonScript(this, path, script_file, id);
+  loaded_scripts_[id] = ret; // Used by RegisterNativeObject during startup
   if (ret->Init()) {
     return ret;
   }
 
-  ret->Unload();
-  delete ret;
+  DestroyScript(ret);
   return NULL;
 }
 
+void PythonEngine::DestroyScript(Script* script) {
+  loaded_scripts_.remove(script->id());
+  script->Unload();
+  delete script;
+}
+
 void PythonEngine::AddObject(void* object, const _sipTypeDef* type,
                              const char * name) const {
   PyObject* python_object = sip_api_->api_convert_from_type(object, type, NULL);
@@ -139,3 +155,48 @@ void PythonEngine::AddObject(void* object, const _sipTypeDef* type,
 void PythonEngine::AddLogLine(const QString& message, bool error) {
   manager()->AddLogLine("Python", message, error);
 }
+
+Script* PythonEngine::FindScriptMatchingId(const QString& id) const {
+  foreach (const QString& script_id, loaded_scripts_.keys()) {
+    if (script_id == id || id.startsWith(script_id + ".")) {
+      return loaded_scripts_[script_id];
+    }
+  }
+  return NULL;
+}
+
+void PythonEngine::RegisterNativeObject(QObject* object) {
+  // This function is called from Python, we need to figure out which script
+  // called it, so we look at the __package__ variable in the bottom stack
+  // frame.
+
+  PyFrameObject* frame = PyEval_GetFrame();
+  if (!frame) {
+    qWarning() << __PRETTY_FUNCTION__ << "unable to get stack frame";
+    return;
+  }
+  while (frame->f_back) {
+    frame = frame->f_back;
+  }
+
+  PyObject* __package__ = PyMapping_GetItemString(
+      frame->f_globals, const_cast<char*>("__package__"));
+  if (!__package__) {
+    qWarning() << __PRETTY_FUNCTION__ << "unable to get __package__";
+    return;
+  }
+
+  QString package = PyString_AsString(__package__);
+  Py_DECREF(__package__);
+  package.remove(QString(kModulePrefix) + ".");
+
+  Script* script = FindScriptMatchingId(package);
+  if (!script) {
+    qWarning() << __PRETTY_FUNCTION__ << "unable to find script for package" << package;
+    return;
+  }
+
+  // Finally got the script - tell it about this object so it will get destroyed
+  // when the script is unloaded.
+  script->AddNativeObject(object);
+}

src/scripting/python/pythonengine.h

@@ -27,6 +27,9 @@ struct _sipTypeDef;
 class PythonEngine : public LanguageEngine {
 public:
   PythonEngine(ScriptManager* manager);
+  ~PythonEngine();
+
+  static PythonEngine* instance() { return sInstance; }
 
   static const char* kModulePrefix;
 
@@ -35,20 +38,32 @@ public:
 
   Script* CreateScript(const QString& path, const QString& script_file,
                        const QString& id);
+  void DestroyScript(Script* script);
 
   const _sipAPIDef* sip_api() const { return sip_api_; }
 
   void AddLogLine(const QString& message, bool error = false);
+  void RegisterNativeObject(QObject* object);
 
 private:
   static const _sipAPIDef* GetSIPApi();
   void AddObject(void* object, const _sipTypeDef* type, const char* name) const;
 
-private:
-  bool initialised_;
+  // Looks for a loaded script whose ID either exactly matches id, or matches
+  // some string at the start of id followed by a dot.  For example,
+  // FindScriptMatchingId("foo") and FindScriptMatchingId("foo.bar") would both
+  // match a Script with an ID of foo, but FindScriptMatchingId("foobar")
+  // would not.
+  Script* FindScriptMatchingId(const QString& id) const;
 
+private:
+  static PythonEngine* sInstance;
+
+  bool initialised_;
   _object* clementine_module_;
   const _sipAPIDef* sip_api_;
+
+  QMap<QString, Script*> loaded_scripts_;
 };
 
 #endif // PYTHONENGINE_H

src/scripting/python/pythonscript.cpp

@@ -111,7 +111,10 @@ bool PythonScript::Unload() {
   foreach (const QString& key, keys_to_delete) {
     PyDict_DelItemString(modules, key.toAscii().constData());
   }
-
   PyEval_ReleaseLock();
+
+  // Delete any native objects this script created
+  qDeleteAll(native_objects_);
+
   return true;
 }

src/scripting/script.cpp

@@ -30,3 +30,7 @@ Script::Script(LanguageEngine* language, const QString& path,
 
 Script::~Script() {
 }
+
+void Script::AddNativeObject(QObject* object) {
+  native_objects_ << object;
+}

src/scripting/script.h

@@ -18,6 +18,7 @@
 #ifndef SCRIPT_H
 #define SCRIPT_H
 
+#include <QList>
 #include <QString>
 
 #include <boost/scoped_ptr.hpp>
@@ -25,6 +26,8 @@
 class LanguageEngine;
 class ScriptInterface;
 
+class QObject;
+
 class Script {
 public:
   Script(LanguageEngine* language, const QString& path,
@@ -37,15 +40,20 @@ public:
   const QString& id() const { return id_; }
   ScriptInterface* interface() const { return interface_.get(); }
 
+  // The script can "own" QObjects like QActions that must be deleted (and
+  // removed from the UI, etc.) when the script is unloaded.
+  void AddNativeObject(QObject* object);
+
   virtual bool Init() = 0;
   virtual bool Unload() = 0;
 
 protected:
-  boost::scoped_ptr<ScriptInterface> interface_;
+  QList<QObject*> native_objects_;
 
 private:
   Q_DISABLE_COPY(Script);
 
+  boost::scoped_ptr<ScriptInterface> interface_;
   LanguageEngine* language_;
   QString path_;
   QString script_file_;

src/scripting/scriptinterface.cpp

@@ -19,6 +19,8 @@
 #include "script.h"
 #include "scriptinterface.h"
 
+#include <QDebug>
+
 ScriptInterface::ScriptInterface(Script* script, QObject* parent)
   : QObject(parent),
     script_(script)
@@ -28,7 +30,3 @@ ScriptInterface::ScriptInterface(Script* script, QObject* parent)
 void ScriptInterface::ShowSettingsDialog() {
   emit SettingsDialogRequested();
 }
-
-void ScriptInterface::AddLogLine(const QString& message, bool error) {
-  script_->language()->manager()->AddLogLine(script_->id(), message, error);
-}

src/scripting/scriptinterface.h

@@ -35,9 +35,6 @@ public slots:
   // Callable by C++
   void ShowSettingsDialog();
 
-  // Callable by the script
-  void AddLogLine(const QString& message, bool error = false);
-
 signals:
   // Scripts should connect to this and show a settings dialog
   void SettingsDialogRequested();

src/scripting/scriptmanager.cpp

@@ -20,6 +20,7 @@
 #include "script.h"
 #include "scriptinterface.h"
 #include "scriptmanager.h"
+#include "uiinterface.h"
 #include "core/utilities.h"
 
 #ifdef HAVE_SCRIPTING_PYTHON
@@ -36,7 +37,8 @@ const char* ScriptManager::kIniFileName = "script.ini";
 const char* ScriptManager::kIniSettingsGroup = "Script";
 
 ScriptManager::ScriptManager(QObject* parent)
-  : QAbstractListModel(parent)
+  : QAbstractListModel(parent),
+    ui_interface_(new UIInterface(this))
 {
 #ifdef HAVE_SCRIPTING_PYTHON
   engines_ << new PythonEngine(this);
@@ -48,8 +50,7 @@ ScriptManager::ScriptManager(QObject* parent)
 ScriptManager::~ScriptManager() {
   foreach (const ScriptInfo& info, info_) {
     if (info.loaded_) {
-      info.loaded_->Unload();
-      delete info.loaded_;
+      info.loaded_->language()->DestroyScript(info.loaded_);
     }
   }
 
@@ -244,8 +245,7 @@ void ScriptManager::Disable(const QModelIndex& index) {
   if (!info->loaded_)
     return;
 
-  info->loaded_->Unload();
-  delete info->loaded_;
+  info->loaded_->language()->DestroyScript(info->loaded_);
   info->loaded_ = NULL;
 
   enabled_scripts_.remove(info->id_);

src/scripting/scriptmanager.h

@@ -27,6 +27,7 @@ class LanguageEngine;
 class Player;
 class PlaylistManager;
 class Script;
+class UIInterface;
 
 class ScriptManager : public QAbstractListModel {
   Q_OBJECT
@@ -68,6 +69,7 @@ public:
 
   void Init(const GlobalData& data);
   const GlobalData& data() const { return data_; }
+  UIInterface* ui() const { return ui_interface_; }
 
   void Enable(const QModelIndex& index);
   void Disable(const QModelIndex& index);
@@ -130,6 +132,7 @@ private:
 
   // Things available to scripts
   GlobalData data_;
+  UIInterface* ui_interface_;
 };
 
 #endif // SCRIPTMANAGER_H

src/scripting/uiinterface.cpp

@@ -0,0 +1,95 @@
+/* This file is part of Clementine.
+   Copyright 2010, David Sansome <me@davidsansome.com>
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "uiinterface.h"
+
+#include <QAction>
+#include <QMenu>
+#include <QtDebug>
+
+UIInterface::UIInterface(QObject* parent)
+  : QObject(parent)
+{
+}
+
+void UIInterface::RegisterActionLocation(const QString& id, QMenu* menu, QAction* before) {
+  if (locations_.contains(id)) {
+    qDebug() << __PRETTY_FUNCTION__
+             << "A location with ID" << id << "was already registered";
+    return;
+  }
+
+  locations_[id] = Location(menu, before);
+
+  connect(menu, SIGNAL(destroyed()), SLOT(MenuDestroyed()));
+  if (before) {
+    connect(before, SIGNAL(destroyed()), SLOT(MenuActionDestroyed()));
+  }
+
+  // Add any actions that were waiting
+  foreach (const IdAndAction& id_action, pending_actions_) {
+    if (id_action.first == id) {
+      DoAddAction(id_action.first, id_action.second);
+      pending_actions_.removeAll(id_action);
+    }
+  }
+}
+
+void UIInterface::AddAction(const QString& id, QAction* action) {
+  if (locations_.contains(id)) {
+    DoAddAction(id, action);
+  } else {
+    // Maybe that part of the UI hasn't been lazy created yet
+    pending_actions_ << IdAndAction(id, action);
+    connect(action, SIGNAL(destroyed()), SLOT(ActionDestroyed()));
+  }
+}
+
+void UIInterface::DoAddAction(const QString& id, QAction* action) {
+  const Location& location = locations_[id];
+
+  if (location.menu_) {
+    location.menu_->insertAction(location.before_, action);
+  }
+}
+
+void UIInterface::MenuDestroyed() {
+  QMenu* menu = qobject_cast<QMenu*>(sender());
+  foreach (const QString& id, locations_.keys()) {
+    if (locations_[id].menu_ == menu) {
+      locations_.remove(id);
+    }
+  }
+}
+
+void UIInterface::MenuActionDestroyed() {
+  QAction* action = qobject_cast<QAction*>(sender());
+  foreach (const QString& id, locations_.keys()) {
+    if (locations_[id].before_ == action) {
+      locations_.remove(id);
+    }
+  }
+}
+
+void UIInterface::ActionDestroyed() {
+  QAction* action = qobject_cast<QAction*>(sender());
+  foreach (const IdAndAction& id_action, pending_actions_) {
+    if (id_action.second == action) {
+      pending_actions_.removeAll(id_action);
+    }
+  }
+}

src/scripting/uiinterface.h

@@ -0,0 +1,64 @@
+/* This file is part of Clementine.
+   Copyright 2010, David Sansome <me@davidsansome.com>
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef UIINTERFACE_H
+#define UIINTERFACE_H
+
+#include <QMap>
+#include <QObject>
+#include <QPair>
+
+class QAction;
+class QMenu;
+
+class UIInterface : public QObject {
+  Q_OBJECT
+
+public:
+  UIInterface(QObject* parent = 0);
+
+  // Called from C++
+  void RegisterActionLocation(const QString& id, QMenu* menu, QAction* before);
+
+  // Called from scripts
+  void AddAction(const QString& id, QAction* action);
+
+private slots:
+  void MenuDestroyed();
+  void MenuActionDestroyed();
+
+  void ActionDestroyed();
+
+private:
+  struct Location {
+    Location() {}
+    Location(QMenu* menu, QAction* before) : menu_(menu), before_(before) {}
+
+    QMenu* menu_;
+    QAction* before_;
+  };
+
+  typedef QPair<QString, QAction*> IdAndAction;
+
+  void DoAddAction(const QString& id, QAction* action);
+
+private:
+  QList<IdAndAction> pending_actions_;
+  QMap<QString, Location> locations_;
+};
+
+#endif // UIINTERFACE_H

src/ui/mainwindow.cpp

@@ -99,6 +99,7 @@
 #ifdef HAVE_SCRIPTING
 # include "scripting/scriptdialog.h"
 # include "scripting/scriptmanager.h"
+# include "scripting/uiinterface.h"
 #endif
 
 #include <QCloseEvent>
@@ -640,12 +641,12 @@ MainWindow::MainWindow(QWidget* parent)
 #endif
 
 #ifdef HAVE_SCRIPTING
+  scripts_->ui()->RegisterActionLocation("help_menu", ui_->menu_help, NULL);
   scripts_->Init(ScriptManager::GlobalData(player_, playlists_));
   connect(ui_->action_script_manager, SIGNAL(triggered()), SLOT(ShowScriptDialog()));
 #else
   ui_->action_script_manager->setEnabled(false);
 #endif
-
 }
 
 MainWindow::~MainWindow() {

src/ui/mainwindow.ui

@@ -393,7 +393,7 @@
      <height>23</height>
     </rect>
    </property>
-   <widget class="QMenu" name="menuMusic">
+   <widget class="QMenu" name="menu_music">
     <property name="title">
      <string>Music</string>
     </property>
@@ -411,7 +411,7 @@
     <addaction name="separator"/>
     <addaction name="action_quit"/>
    </widget>
-   <widget class="QMenu" name="menuPlaylist">
+   <widget class="QMenu" name="menu_playlist">
     <property name="title">
      <string>Playlist</string>
     </property>
@@ -430,14 +430,14 @@
     <addaction name="action_clear_playlist"/>
     <addaction name="action_shuffle"/>
    </widget>
-   <widget class="QMenu" name="menuHelp">
+   <widget class="QMenu" name="menu_help">
     <property name="title">
      <string>Help</string>
     </property>
     <addaction name="action_about"/>
     <addaction name="action_about_qt"/>
    </widget>
-   <widget class="QMenu" name="menuExtras">
+   <widget class="QMenu" name="menu_extras">
     <property name="title">
      <string>Extras</string>
     </property>
@@ -445,7 +445,7 @@
     <addaction name="action_hypnotoad"/>
     <addaction name="action_kittens"/>
    </widget>
-   <widget class="QMenu" name="menuTools">
+   <widget class="QMenu" name="menu_tools">
     <property name="title">
      <string>Tools</string>
     </property>
@@ -460,11 +460,11 @@
     <addaction name="separator"/>
     <addaction name="action_configure"/>
    </widget>
-   <addaction name="menuMusic"/>
-   <addaction name="menuPlaylist"/>
-   <addaction name="menuTools"/>
-   <addaction name="menuExtras"/>
-   <addaction name="menuHelp"/>
+   <addaction name="menu_music"/>
+   <addaction name="menu_playlist"/>
+   <addaction name="menu_tools"/>
+   <addaction name="menu_extras"/>
+   <addaction name="menu_help"/>
   </widget>
   <action name="action_previous_track">
    <property name="text">