News:

As usual while waiting for the next release - don't forget to check the nightly builds in the forum.

Main Menu

Allowing Plugin Interdependency and Improving Plugin Management

Started by dmoore, November 10, 2013, 06:53:32 PM

Previous topic - Next topic
Print
Go Down Pages 1 2 3 All
User actions

ollydbg

#30
February 16, 2016, 06:26:56 AM Last Edit: February 16, 2016, 06:31:28 AM by ollydbg
On my test, install a bundle(such as a file named ProjectsImporter-1.1.cbplugin which I exported from C::B) failed in function bool PluginManager::InstallPlugin(const wxString& pluginName, bool forAllUsers, bool askForConfirmation)
Code Select

    // extract plugin from bundle
    if (!ExtractFile(actualName,
                    localName,
                    pluginFilename))
        return false;

Not sure the reason.

EDIT: uninstall an unenabled plugin also failed without any dialog or message box.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

ollydbg

#31
February 16, 2016, 06:37:53 AM
Quote from: ollydbg on February 16, 2016, 06:26:56 AM
On my test, install a bundle(such as a file named ProjectsImporter-1.1.cbplugin which I exported from C::B) failed in function bool PluginManager::InstallPlugin(const wxString& pluginName, bool forAllUsers, bool askForConfirmation)
Code Select

    // extract plugin from bundle
    if (!ExtractFile(actualName,
                    localName,
                    pluginFilename))
        return false;

Not sure the reason.

EDIT: uninstall an unenabled plugin also failed without any dialog or message box.

For an disabled plugin.
Code Select

void PluginsConfigurationDlg::OnUninstall(cb_unused wxCommandEvent& event)
{
    wxListCtrl* list = XRCCTRL(*this, "lstPlugins", wxListCtrl);
    if (list->GetSelectedItemCount() == 0)
        return;

    wxBusyCursor busy;

    long sel = -1;
    wxString failure;
    while (true)
    {
        sel = list->GetNextItem(sel, wxLIST_NEXT_ALL, wxLIST_STATE_SELECTED);
        if (sel == -1)
            break;

        PluginElement* elem = (PluginElement*)list->GetItemData(sel);
        if (elem && elem->plugin)
        {
            if (!Manager::Get()->GetPluginManager()->UninstallPlugin(elem))
                failure << elem->info.title << _T('\n');
        }
    }

    FillList();
    if (!failure.IsEmpty())
        cbMessageBox(_("One or more plugins were not un-installed successfully:\n\n") + failure, _("Warning"), wxICON_WARNING, this);
}


elem->plugin is NULL, but we still need to uninstall it, right?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

ollydbg

#32
February 16, 2016, 06:40:24 AM
Quote from: dmoore on November 10, 2013, 07:33:14 PM
Patch updated to handle safemode
:o, you have version 2 of your patch here, but I'm testing your version 1 patch...
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

ollydbg

#33
February 16, 2016, 07:51:15 AM
My new patch is out, which is based on dmoore's version 2 patch, it can uninstall an unloaded plugin.
Code::Blocks / Tickets / #300 Only load plugin shared library objects for plugins that are enabled

But I still have an issue that I can't install an exported plugin bundle, reported in my previous posts.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

ollydbg

#34
February 16, 2016, 08:53:39 AM
Quote from: ollydbg on February 16, 2016, 07:51:15 AM
But I still have an issue that I can't install an exported plugin bundle, reported in my previous posts.
OK, I found the reason, I see that in my exported plugin bundle, the dll and zip file name are all in lowercase, but the bundle name is "ProjectsImporter-1.1.cbplugin", so that the name are not matched. I'm bulding C::B against wx trunk(wx31)

This does not happens in our 16.01 release(it will generated ProjectsImporter.dll and ProjectsImporter.zip file)
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

ollydbg

#35
February 16, 2016, 09:11:15 AM Last Edit: February 16, 2016, 09:17:33 AM by ollydbg
Return to the original issue when install a new plugin
Code Select

    ScanForPlugins(pluginDir);
    LoadAllPlugins();
    cbPlugin* plugin = FindPluginByName(basename); //THIS DOESN'T WORK RIGHT...
    const PluginInfo* info = GetPluginInfo(plugin);


Here, I see the plugin is NULL. Because I think LoadAllPlugins() only load the manifest(xml), but the dll is not loaded yet.

EDIT:
Maybe, we need to just Load the plugin as in the code:
Code Select

void PluginsConfigurationDlg::OnToggle(wxCommandEvent& event)
{
    wxListCtrl* list = XRCCTRL(*this, "lstPlugins", wxListCtrl);
    if (list->GetSelectedItemCount() == 0)
        return;
    bool isEnable = event.GetId() == XRCID("btnEnable");

    wxBusyCursor busy;

    wxProgressDialog pd(wxString::Format(_("%s plugin(s)"), isEnable ? _("Enabling") : _("Disabling")),
                        _T("A description wide enough for the dialog ;)"),
                        list->GetSelectedItemCount(),
                        this,
                        wxPD_AUTO_HIDE | wxPD_APP_MODAL | wxPD_CAN_ABORT);

    int count = 0;
    long sel = -1;
    bool skip = false;
    while (true)
    {
        sel = list->GetNextItem(sel, wxLIST_NEXT_ALL, wxLIST_STATE_SELECTED);
        if (sel == -1)
            break;

        PluginElement* elem = (PluginElement*)list->GetItemData(sel);
        if (elem)
        {
            pd.Update(++count,
                        wxString::Format(_("%s \"%s\"..."), isEnable ? _("Enabling") : _("Disabling"), elem->info.title.c_str()),
                        &skip);
            if (skip)
                break;

            if (!isEnable && elem->plugin && elem->plugin->IsAttached())
            {
                Manager::Get()->GetPluginManager()->DetachPlugin(elem->plugin);
                Manager::Get()->GetPluginManager()->UnloadPlugin(elem->plugin);
            }
            else if (isEnable && !(elem->plugin && elem->plugin->IsAttached()))
            {
                if (!elem->plugin)
                    Manager::Get()->GetPluginManager()->LoadPlugin(elem->fileName, elem);
                Manager::Get()->GetPluginManager()->AttachPlugin(elem->plugin, true); // ignore safe-mode here
            }
            else
                continue;


See the above code:
Code Select

                if (!elem->plugin)
                    Manager::Get()->GetPluginManager()->LoadPlugin(elem->fileName, elem);

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

ollydbg

#36
February 17, 2016, 02:32:11 AM
I have find the solution to solve the "THIS DOESN'T WORK RIGHT" problem in dmoore's original v2 patch.

Please have a look and test the v4.patch in Code::Blocks / Tickets / #300 Only load plugin shared library objects for plugins that are enabled.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

blauzahn

#37
February 17, 2016, 08:00:29 AM

just glanced over the patch.

Code Select

void PluginManager::AddUnloadedPluginElem(const wxString &path, const PluginInfo &info)
{
    PluginElement *plugElem  = new PluginElement;
    plugElem->fileName = path;
    plugElem->info = info;
    plugElem->library = 0;
    plugElem->freeProc = 0;
    plugElem->plugin = 0;
    m_Plugins.Add(plugElem);
}


I would prefer nullptr instead of 0. At other places in the patch as well.

The method name AddUnloadedPluginElement reveals, that it is doing actually 2 things:
Creating a PluginElement and adding it. To me, the creation details belong to PluginElement.
I'd convert it into a ctor or maybe into a static method to give it a name like:

Code Select

static PluginElement PluginElement::unloaded(const wxString &path, const PluginInfo &info);


That way, the void method wil evaporate into a mere call :

Code Select

m_Plugins.Add( new PluginElement::unloaded(path, info));


where at least the creation is more exception safe and the newly created element is given
directly to the array. I always shiver when I see naked new and delete.

oBFusCATed

#38
February 17, 2016, 09:36:09 AM
Quote from: blauzahn on February 17, 2016, 08:00:29 AM
I always shiver when I see naked new and delete.
Welcome to the real world outside of the committee...
Except for the nullptrs everything else is not that bad in this function.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

frithjofh

#39
February 17, 2016, 10:29:21 AM
@obfuscated

is there a special reason, you don't implement and use a constructor for PluginElement taking a String and a PluginInfo and setting the other members to nullptr in the initializer list?
architect with some spare time  -  c::b compiled from last svn  -   openSuSE leap x86_64  -  AMD FX-4100

oBFusCATed

#40
February 17, 2016, 10:37:41 AM
Because it doesn't matter that much.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]
Print
Go Up Pages 1 2 3 All
User actions