preferences: Classify plugins in System and User plugins
AbandonedPublic

Authored by cfoch on Jul 31 2017, 11:39 AM.

Details

Reviewers
thiblahute
aleb
Summary

Depends on D1789

Diff Detail

Repository
rPTV Pitivi
Branch
T3193-pluginmanager
cfoch created this revision.Jul 31 2017, 11:39 AM
aleb requested changes to this revision.Jul 31 2017, 12:20 PM
aleb added inline comments.
pitivi/dialogs/prefs.py
676–681

This should be item, not model. Change also the class name - PluginItem

686

FYI, this should also test self's type, otherwise system_plugin_1 < system_plugin_2 and system_plugin_2 < system_plugin_1 which could confuse the sort algorithm.

688

If you create first a list of row items, it would be more efficient, no need to recompute get_plugin_type() for every sort, and you won't need this _cmp function, you can use something like this to sort by plugin_type and then alphabetically.

for item in sorted(items, key=attrgetter("plugin_type", "name")):
    self.append(model)
771–795

Better move the get_plugin_type functionality to the class you use for the rows so you can use directly row.get_plugin_type() or something like that.

773

previous_type

780

Instead of if..elif you can have a single if previous_type != current_type: --- if you move the strings to the PluginType class.

pitivi/pluginmanager.py
89

remove parenthesis

This revision now requires changes to proceed.Jul 31 2017, 12:20 PM
cfoch marked an inline comment as done.Aug 1 2017, 4:34 AM
cfoch added inline comments.
pitivi/dialogs/prefs.py
686

What do you mean by "self" type. This is a static method, so there is no 'self'. By returning 1 when it is not a System Plugin and -1 when it is a system plugin, I say that System plugins goes before whichever other type.
If there are two System Plugins, I am not sure which order Python chooses, but whichever the order was, they will be always before User Plugins.

I think now that instead of returning -1 and 1, I could just return the type, since it is an enum. What do you think?

cfoch added inline comments.Aug 1 2017, 4:45 AM
pitivi/dialogs/prefs.py
771–795

I was thinking that it makes more sense that the PluginManager tells you what type of plugin is certain plugin, just because this is the "manager". You would also have an easy access through objects to that info. The decision is on you.

cfoch added inline comments.Aug 1 2017, 5:16 AM
pitivi/dialogs/prefs.py
688

The sort is executed only once every time you open the PreferencesDialog.
...Sorting it by plugin_type and then alphabetically makes total sense to me.

cfoch marked 2 inline comments as done.Aug 1 2017, 5:17 AM
aleb added inline comments.Aug 1 2017, 7:53 PM
pitivi/dialogs/prefs.py
686

Oh, I thought that because you return -1 and 1, it's actually a compare method. Not to mention the name. :P

cfoch updated this revision to Diff 6196.Aug 6 2017, 9:16 AM
cfoch marked an inline comment as done.
aleb accepted this revision.Aug 14 2017, 3:24 AM
This revision is now accepted and ready to land.Aug 14 2017, 3:24 AM