PICARD-1177 : Add a option to remove plugins #804

vishichoudhary · 2018-01-20T14:03:34Z

Summary

This is a…
- Bug fix
- Feature addition
- Refactoring
- Minor / simple change (like a typo)
- Other
This will add a button to uninstall a plugin:

Problem

JIRA ticket (optional): PICARD-1177

Solution

Added a new column in Plugin gui , to uninstall a plugin . Added a confirmation box too

Action

samj1912 · 2018-01-20T14:10:45Z

picard/ui/options/plugins.py

@@ -269,6 +269,22 @@ def download_button_process():
            else:
                label = _("Installed")
            self.ui.plugins.setItemWidget(item, 2, QtWidgets.QLabel(label))
+            button = QtWidgets.QPushButton(_("Uninstall"))
+            button.setMaximumHeight(button.fontMetrics().boundingRect(label).height() + 7)


Why this arbitrary +7? Do not make UI changes in non ui_*.py files.

Please address this.

samj1912 · 2018-01-20T14:11:08Z

picard/ui/options/plugins.py

+            def plugin_remove():
+                self.ui.plugins.setCurrentItem(item)
+                plugin = self.items[self.ui.plugins.selectedItems()[0]]
+                self.tagger.pluginmanager.remove_plugin(plugin.module_name + '.zip')


It isn't always a zip. It could also be a single py file or a folder.

samj1912 · 2018-01-20T14:14:10Z

picard/ui/options/plugins.py

+            button.setMaximumHeight(button.fontMetrics().boundingRect(label).height() + 7)
+            self.ui.plugins.setItemWidget(item, 3, button)
+
+            def plugin_remove():


Do not introduce function definitions inside other functions.

samj1912 · 2018-01-20T14:15:00Z

picard/ui/options/plugins.py

+                msgbox.setStandardButtons(QtWidgets.QMessageBox.Ok)
+                msgbox.setDefaultButton(QtWidgets.QMessageBox.Ok)
+                msgbox.exec_()
+            button.released.connect(plugin_remove)


You might want to ask for user confirmation here.

samj1912 · 2018-01-20T14:15:11Z

picard/ui/options/plugins.py

+                self.tagger.pluginmanager.remove_plugin(plugin.module_name + '.zip')
+                msgbox = QtWidgets.QMessageBox(self)
+                msgbox.setText(
+                    _("The plugin '%s' will be remove on next run of Picard.")


s/remove/removed

samj1912 · 2018-01-20T14:15:55Z

Please fix the PR template and link to the Jira ticket.

samj1912 · 2018-01-21T13:35:36Z

This is how my picard looks. Why do we need an extra Remove column? you could simply modify the install button to become the uninstall buttons once its installed.Or better yet, keep Status as is and change its value to Installed/ Not installed and keep the install/uninstall button on the side.

@chhavijustme any thoughts on this?

samj1912 · 2018-01-21T13:37:56Z

picard/ui/options/plugins.py

@@ -269,6 +269,22 @@ def download_button_process():
            else:
                label = _("Installed")
            self.ui.plugins.setItemWidget(item, 2, QtWidgets.QLabel(label))
+            button = QtWidgets.QPushButton(_("Uninstall"))
+            button.setMaximumHeight(button.fontMetrics().boundingRect(label).height() + 7)


Please address this.

samj1912 · 2018-01-21T13:41:33Z

picard/ui/options/plugins.py

+            buttonReply = QtWidgets.QMessageBox.question(self,'Uninstall?', "Do you want to uninstall?",
+                    QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No, QtWidgets.QMessageBox.No)
+            if buttonReply == QtWidgets.QMessageBox.Yes:
+                if os.path.isfile(USER_PLUGIN_DIR+plugin.module_name+'.zip'):


I cannot fathom why you need to add extensions to the plugin names. It will only cause erroneous results with remove_plugin. Remove_plugin just expects the plugin name by default. Nothing else. It handles stripping of extensions and deletion of files.

See https://github.com/metabrainz/picard/blob/master/picard/plugin.py#L351

I think we can do something like this.

Instead of course, following=> uninstall and follow=>install. The important thing here is to keep it looking like a button so that people know it can be clicked.

@vishichoudhary

I don't know the case here, but if update is available for only a few plugins at a time,
You can add an "update" link in front of all plugins whose update is available.

If an update is available for all the plugins all the time, then having install/uninstall button and an update button makes more sense.

@chhavijustme what about having only one button for install / uninstall and update automatically?

vishichoudhary · 2018-01-21T13:50:02Z

@chhavijustme @samj1912 There is already a second operation UPDATE on it so i can't add REMOVE on the same button. :-(

vishichoudhary · 2018-01-21T13:59:40Z

@samj1912 , i am extracting the module name only, then i am checking that whether this moudule name exists as .zip or .py or as a folder?

samj1912 · 2018-01-21T14:06:04Z

@vishichoudhary yes and you don't need to do any of that. Simply the last line will suffice.
self.tagger.pluginmanager.remove_plugin(plugin.module_name)

You don't need to do the extension checks.

vishichoudhary · 2018-01-21T14:08:51Z

ok @samj1912 i will make changes accordingly. :-)

vishichoudhary · 2018-01-21T14:19:47Z

@chhavijustme i didn't get you on your last point. Can you please explain it bit more? :-)

zas · 2018-01-21T22:03:37Z

picard/ui/ui_options_plugins.py

@@ -129,6 +129,7 @@ def retranslateUi(self, PluginsOptionsPage):
        self.plugins.headerItem().setText(0, _("Name"))
        self.plugins.headerItem().setText(1, _("Version"))
        self.plugins.headerItem().setText(2, _("Status"))
+        self.plugins.headerItem().setText(3, _("Remove"))


I would prefer Uninstall here, if the actual effect for the user is a plugin uninstallation
And since only an installed plugin can be uninstalled, only one button is needed.

vishichoudhary · 2018-01-22T17:23:07Z

Messed up with this branch, as it's on my master branch , will pr again with new branch and with some more fixes. Sorry :-(

samj1912 requested changes Jan 20, 2018

View reviewed changes

samj1912 requested changes Jan 21, 2018

View reviewed changes

zas requested changes Jan 22, 2018

View reviewed changes

vishichoudhary closed this Jan 22, 2018

vishichoudhary mentioned this pull request Jan 23, 2018

PICARD-1177 : Add a option to remove plugin #809

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PICARD-1177 : Add a option to remove plugins #804

PICARD-1177 : Add a option to remove plugins #804

vishichoudhary commented Jan 20, 2018 •

edited by samj1912

samj1912 Jan 20, 2018

samj1912 Jan 21, 2018

samj1912 Jan 20, 2018

samj1912 Jan 20, 2018

samj1912 Jan 20, 2018

samj1912 Jan 20, 2018

samj1912 commented Jan 20, 2018

samj1912 commented Jan 21, 2018 •

edited

samj1912 Jan 21, 2018

samj1912 Jan 21, 2018

chhavijustme Jan 21, 2018

chhavijustme Jan 21, 2018

vishichoudhary Jan 21, 2018

vishichoudhary commented Jan 21, 2018 •

edited

vishichoudhary commented Jan 21, 2018

samj1912 commented Jan 21, 2018

vishichoudhary commented Jan 21, 2018

vishichoudhary commented Jan 21, 2018

zas Jan 21, 2018

vishichoudhary commented Jan 22, 2018

PICARD-1177 : Add a option to remove plugins #804

PICARD-1177 : Add a option to remove plugins #804

Conversation

vishichoudhary commented Jan 20, 2018 • edited by samj1912

Summary

Problem

Solution

Action

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samj1912 commented Jan 20, 2018

samj1912 commented Jan 21, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishichoudhary commented Jan 21, 2018 • edited

vishichoudhary commented Jan 21, 2018

samj1912 commented Jan 21, 2018

vishichoudhary commented Jan 21, 2018

vishichoudhary commented Jan 21, 2018

Choose a reason for hiding this comment

vishichoudhary commented Jan 22, 2018

vishichoudhary commented Jan 20, 2018 •

edited by samj1912

samj1912 commented Jan 21, 2018 •

edited

vishichoudhary commented Jan 21, 2018 •

edited