From 139b99d371bde6c04eddd0079045debcc16cc7bf Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Thu, 8 Sep 2022 22:19:30 -0500 Subject: [PATCH] Addon Manager: Cleanup --- src/App/Metadata.cpp | 6 +- src/App/MetadataPy.xml | 7 ++ src/App/MetadataPyImp.cpp | 14 ++- src/Mod/AddonManager/CMakeLists.txt | 1 - src/Mod/AddonManager/addonmanager_devmode.py | 3 +- .../addonmanager_devmode_add_content.py | 110 +----------------- .../addonmanager_devmode_dependencies.py | 21 ---- .../addonmanager_devmode_licenses_table.py | 10 +- .../addonmanager_devmode_people_table.py | 3 + .../addonmanager_devmode_predictor.py | 58 +++++---- .../addonmanager_devmode_validators.py | 13 +-- 11 files changed, 77 insertions(+), 169 deletions(-) delete mode 100644 src/Mod/AddonManager/addonmanager_devmode_dependencies.py diff --git a/src/App/Metadata.cpp b/src/App/Metadata.cpp index ec24dd1602..f43aa494f2 100644 --- a/src/App/Metadata.cpp +++ b/src/App/Metadata.cpp @@ -655,7 +655,9 @@ void Metadata::appendToElement(DOMElement* root) const { appendSimpleXMLNode(root, "name", _name); appendSimpleXMLNode(root, "description", _description); - appendSimpleXMLNode(root, "version", _version.str()); + if (_version != Meta::Version()) + // Only append version if it's not 0.0.0 + appendSimpleXMLNode(root, "version", _version.str()); for (const auto& maintainer : _maintainer) { auto element = appendSimpleXMLNode(root, "maintainer", maintainer.name); @@ -977,6 +979,8 @@ Meta::Version::Version(const std::string& versionString) : std::string Meta::Version::str() const { + if (*this == Meta::Version()) + return ""; std::ostringstream stream; stream << major << "." << minor << "." << patch << suffix; return stream.str(); diff --git a/src/App/MetadataPy.xml b/src/App/MetadataPy.xml index 2847860864..712b5cf15d 100644 --- a/src/App/MetadataPy.xml +++ b/src/App/MetadataPy.xml @@ -365,6 +365,13 @@ Add a new File. Remove the File. + + + + write(filename)\n +Write the metadata to the given file as XML data. + + public: diff --git a/src/App/MetadataPyImp.cpp b/src/App/MetadataPyImp.cpp index 0a7e8d1033..c6ce51ffc3 100644 --- a/src/App/MetadataPyImp.cpp +++ b/src/App/MetadataPyImp.cpp @@ -139,7 +139,7 @@ void MetadataPy::setVersion(Py::Object args) const char *name = nullptr; if (!PyArg_Parse(args.ptr(), "z", &name)) throw Py::Exception(); - if (name) + if (name && name[0] != '\0') getMetadataPtr()->setVersion(App::Meta::Version(std::string(name))); else getMetadataPtr()->setVersion(App::Meta::Version()); @@ -934,6 +934,18 @@ PyObject *MetadataPy::removeContentItem(PyObject *arg) return Py_None; } +PyObject* MetadataPy::write(PyObject* args) +{ + char *filename = nullptr; + if (!PyArg_ParseTuple(args, "s", &filename)) + return nullptr; + getMetadataPtr()->write(filename); + + Py_INCREF(Py_None); + return Py_None; +} + + PyObject *MetadataPy::getCustomAttributes(const char * /*attr*/) const { return nullptr; diff --git a/src/Mod/AddonManager/CMakeLists.txt b/src/Mod/AddonManager/CMakeLists.txt index 6c60c00e3d..4f7dbd1a63 100644 --- a/src/Mod/AddonManager/CMakeLists.txt +++ b/src/Mod/AddonManager/CMakeLists.txt @@ -9,7 +9,6 @@ SET(AddonManager_SRCS AddonManager.ui addonmanager_devmode.py addonmanager_devmode_add_content.py - addonmanager_devmode_dependencies.py addonmanager_devmode_license_selector.py addonmanager_devmode_licenses_table.py addonmanager_devmode_person_editor.py diff --git a/src/Mod/AddonManager/addonmanager_devmode.py b/src/Mod/AddonManager/addonmanager_devmode.py index 98b1f780e2..b24e0fbc56 100644 --- a/src/Mod/AddonManager/addonmanager_devmode.py +++ b/src/Mod/AddonManager/addonmanager_devmode.py @@ -30,7 +30,6 @@ import FreeCADGui from PySide2.QtWidgets import ( QFileDialog, - QTableWidgetItem, QListWidgetItem, QDialog, QSizePolicy, @@ -90,12 +89,14 @@ class AddonGitInterface: return AddonGitInterface.git_manager.get_last_authors(self.path, 10) return [] +#pylint: disable=too-many-instance-attributes class DeveloperMode: """The main Developer Mode dialog, for editing package.xml metadata graphically.""" def __init__(self): + # In the UI we want to show a translated string for the person type, but the underlying # string must be the one expected by the metadata parser, in English self.person_type_translation = { diff --git a/src/Mod/AddonManager/addonmanager_devmode_add_content.py b/src/Mod/AddonManager/addonmanager_devmode_add_content.py index e05e0d1373..8e0cdb9af7 100644 --- a/src/Mod/AddonManager/addonmanager_devmode_add_content.py +++ b/src/Mod/AddonManager/addonmanager_devmode_add_content.py @@ -41,19 +41,11 @@ from PySide2.QtWidgets import ( from PySide2.QtGui import QIcon from PySide2.QtCore import Qt -from addonmanager_devmode_license_selector import LicenseSelector -from addonmanager_devmode_person_editor import PersonEditor from addonmanager_devmode_validators import ( VersionValidator, NameValidator, PythonIdentifierValidator, ) -from addonmanager_devmode_utilities import ( - populate_people_from_metadata, - populate_licenses_from_metadata, - add_license_row, - add_person_row, -) from addonmanager_devmode_people_table import PeopleTable from addonmanager_devmode_licenses_table import LicensesTable @@ -261,10 +253,10 @@ class AddContent: licenses = [] for row in range(self.dialog.licensesTableWidget.rowCount()): - license = {} - license["name"] = self.dialog.licensesTableWidget.item(row, 0).text - license["file"] = self.dialog.licensesTableWidget.item(row, 1).text() - licenses.append(license) + new_license = {} + new_license["name"] = self.dialog.licensesTableWidget.item(row, 0).text + new_license["file"] = self.dialog.licensesTableWidget.item(row, 1).text() + licenses.append(new_license) self.metadata.License = licenses return (self.dialog.addonKindComboBox.currentData(), self.metadata) @@ -370,98 +362,7 @@ class AddContent: if not self.metadata: self.metadata = FreeCAD.Metadata() dlg = EditDependencies() - result = dlg.exec(self.metadata) - - def _person_selection_changed(self): - """Callback: the current selection in the peopleTableWidget changed""" - items = self.dialog.peopleTableWidget.selectedItems() - if items: - self.dialog.removePersonToolButton.setDisabled(False) - else: - self.dialog.removePersonToolButton.setDisabled(True) - - def _license_selection_changed(self): - """Callback: the current selection in the licensesTableWidget changed""" - items = self.dialog.licensesTableWidget.selectedItems() - if items: - self.dialog.removeLicenseToolButton.setDisabled(False) - else: - self.dialog.removeLicenseToolButton.setDisabled(True) - - def _add_license_clicked(self): - """Callback: The Add License button was clicked""" - license_selector = LicenseSelector(self.current_mod) - short_code, path = license_selector.exec() - if short_code: - add_license_row( - self.dialog.licensesTableWidget.rowCount(), - short_code, - path, - self.path_to_addon, - self.dialog.licensesTableWidget, - ) - - def _remove_license_clicked(self): - """Callback: the Remove License button was clicked""" - items = self.dialog.licensesTableWidget.selectedIndexes() - if items: - # We only support single-selection, so can just pull the row # from - # the first entry - self.dialog.licensesTableWidget.removeRow(items[0].row()) - - def _edit_license(self, item): - """Callback: a license row was double-clicked""" - row = item.row() - short_code = self.dialog.licensesTableWidget.item(row, 0).text() - path = self.dialog.licensesTableWidget.item(row, 1).text() - license_selector = LicenseSelector(self.current_mod) - short_code, path = license_selector.exec(short_code, path) - if short_code: - self.dialog.licensesTableWidget.removeRow(row) - add_license_row( - row, - short_code, - path, - self.path_to_addon, - self.dialog.licensesTableWidget, - ) - - def _add_person_clicked(self): - """Callback: the Add Person button was clicked""" - dlg = PersonEditor() - person_type, name, email = dlg.exec() - if person_type and name: - add_person_row( - self.dialog.peopleTableWidget.rowCount(), - person_type, - name, - email, - self.dialog.peopleTableWidget, - ) - - def _remove_person_clicked(self): - """Callback: the Remove Person button was clicked""" - items = self.dialog.peopleTableWidget.selectedIndexes() - if items: - # We only support single-selection, so can just pull the row # from - # the first entry - self.dialog.peopleTableWidget.removeRow(items[0].row()) - - def _edit_person(self, item): - """Callback: a row in the peopleTableWidget was double-clicked""" - row = item.row() - person_type = self.dialog.peopleTableWidget.item(row, 0).data(Qt.UserRole) - name = self.dialog.peopleTableWidget.item(row, 1).text() - email = self.dialog.peopleTableWidget.item(row, 2).text() - - dlg = PersonEditor() - dlg.setup(person_type, name, email) - person_type, name, email = dlg.exec() - - if person_type and name: - self.dialog.peopleTableWidget.removeRow(row) - add_person_row(row, person_type, name, email, self.dialog.peopleTableWidget) - self.dialog.peopleTableWidget.selectRow(row) + dlg.exec(self.metadata) # Modifies metadata directly class EditTags: @@ -513,6 +414,7 @@ class EditDependencies: ) self.dialog.removeDependencyToolButton.setDisabled(True) + self.metadata = None def exec(self, metadata: FreeCAD.Metadata): """Execute the dialog""" diff --git a/src/Mod/AddonManager/addonmanager_devmode_dependencies.py b/src/Mod/AddonManager/addonmanager_devmode_dependencies.py deleted file mode 100644 index 2c7df548b0..0000000000 --- a/src/Mod/AddonManager/addonmanager_devmode_dependencies.py +++ /dev/null @@ -1,21 +0,0 @@ -# *************************************************************************** -# * * -# * Copyright (c) 2022 FreeCAD Project Association * -# * * -# * This file is part of FreeCAD. * -# * * -# * FreeCAD is free software: you can redistribute it and/or modify it * -# * under the terms of the GNU Lesser General Public License as * -# * published by the Free Software Foundation, either version 2.1 of the * -# * License, or (at your option) any later version. * -# * * -# * FreeCAD 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 * -# * Lesser General Public License for more details. * -# * * -# * You should have received a copy of the GNU Lesser General Public * -# * License along with FreeCAD. If not, see * -# * . * -# * * -# *************************************************************************** diff --git a/src/Mod/AddonManager/addonmanager_devmode_licenses_table.py b/src/Mod/AddonManager/addonmanager_devmode_licenses_table.py index e10d14f477..f05c49ccb7 100644 --- a/src/Mod/AddonManager/addonmanager_devmode_licenses_table.py +++ b/src/Mod/AddonManager/addonmanager_devmode_licenses_table.py @@ -26,7 +26,6 @@ import os from PySide2.QtWidgets import QTableWidgetItem from PySide2.QtGui import QIcon -from PySide2.QtCore import Qt import FreeCAD import FreeCADGui @@ -35,6 +34,7 @@ from addonmanager_devmode_license_selector import LicenseSelector translate = FreeCAD.Qt.translate +#pylint: disable=too-few-public-methods class LicensesTable: """A QTableWidget and associated buttons for managing the list of authors and maintainers.""" @@ -55,6 +55,8 @@ class LicensesTable: self.widget.removeButton.clicked.connect(self._remove_clicked) self.widget.tableWidget.itemSelectionChanged.connect(self._selection_changed) self.widget.tableWidget.itemDoubleClicked.connect(self._edit) + self.metadata = None + self.path_to_addon = "" def show(self, metadata, path_to_addon): """Set up the widget based on incoming metadata""" @@ -68,9 +70,9 @@ class LicensesTable: """Use the passed metadata object to populate the maintainers and authors""" self.widget.tableWidget.setRowCount(0) row = 0 - for license in self.metadata.License: - shortcode = license["name"] - path = license["file"] + for l in self.metadata.License: + shortcode = l["name"] + path = l["file"] self._add_row(row, shortcode, path) row += 1 diff --git a/src/Mod/AddonManager/addonmanager_devmode_people_table.py b/src/Mod/AddonManager/addonmanager_devmode_people_table.py index 08ea9ef4f7..757b17be26 100644 --- a/src/Mod/AddonManager/addonmanager_devmode_people_table.py +++ b/src/Mod/AddonManager/addonmanager_devmode_people_table.py @@ -35,6 +35,8 @@ from addonmanager_devmode_person_editor import PersonEditor translate = FreeCAD.Qt.translate +#pylint: disable=too-few-public-methods + class PeopleTable: """A QTableWidget and associated buttons for managing the list of authors and maintainers.""" @@ -55,6 +57,7 @@ class PeopleTable: self.widget.removeButton.clicked.connect(self._remove_clicked) self.widget.tableWidget.itemSelectionChanged.connect(self._selection_changed) self.widget.tableWidget.itemDoubleClicked.connect(self._edit) + self.metadata = None def show(self, metadata): """Set up the widget based on incoming metadata""" diff --git a/src/Mod/AddonManager/addonmanager_devmode_predictor.py b/src/Mod/AddonManager/addonmanager_devmode_predictor.py index 0c3063f38b..0bbb6f048e 100644 --- a/src/Mod/AddonManager/addonmanager_devmode_predictor.py +++ b/src/Mod/AddonManager/addonmanager_devmode_predictor.py @@ -32,11 +32,13 @@ import datetime import os import FreeCAD -from addonmanager_git import initialize_git, GitFailed, GitManager +from addonmanager_git import initialize_git, GitManager from addonmanager_utilities import get_readme_url translate = FreeCAD.Qt.translate +#pylint: disable=too-few-public-methods + class AddonSlice: """A tiny class to implement duck-typing for the URL-parsing utility functions""" @@ -54,6 +56,7 @@ class Predictor: self.path = None self.metadata = FreeCAD.Metadata() self.license_data = None + self.readme_data = None self.license_file = "" self.git_manager: GitManager = initialize_git() if not self.git_manager: @@ -86,28 +89,26 @@ class Predictor: # is common for there to be multiple entries representing the same human being, # so a passing attempt is made to reconcile: filtered_committers = {} - for key in committers: - emails = committers[key]["email"] + for key,committer in committers.items(): if "github" in key.lower(): # Robotic merge commit (or other similar), ignore continue # Does any other committer share any of these emails? - for other_key in committers: + for other_key,other_committer in committers.items(): if other_key == key: continue - other_emails = committers[other_key]["email"] - for other_email in other_emails: - if other_email in emails: + for other_email in other_committer["email"]: + if other_email in committer["email"]: # There is overlap in the two email lists, so this is probably the # same author, with a different name (username, pseudonym, etc.) - if not committers[key]["aka"]: - committers[key]["aka"] = set() - committers[key]["aka"].add(other_key) - committers[key]["count"] += committers[other_key]["count"] - committers[key]["email"].combine(committers[other_key]["email"]) - committers.remove(other_key) + if not committer["aka"]: + committer["aka"] = set() + committer["aka"].add(other_key) + committer["count"] += other_committer["count"] + committer["email"].combine(other_committer["email"]) + committers.pop(other_key) break - filtered_committers[key] = committers[key] + filtered_committers[key] = committer maintainers = [] for name, info in filtered_committers.items(): if "aka" in info: @@ -181,8 +182,8 @@ class Predictor: def _predict_license(self): """Predict the license based on any existing license file.""" - # These are processed in order, so the BSD 3 clause must come before the 2, for example, because - # the only difference between them is the additional clause. + # These are processed in order, so the BSD 3 clause must come before the 2, for example, + # because the only difference between them is the additional clause. known_strings = { "Apache-2.0": ( "Apache License, Version 2.0", @@ -190,15 +191,20 @@ class Predictor: ), "BSD-3-Clause": ( "The 3-Clause BSD License", - "3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission.", + "3. Neither the name of the copyright holder nor the names of its contributors \ + may be used to endorse or promote products derived from this software without \ + specific prior written permission.", ), "BSD-2-Clause": ( "The 2-Clause BSD License", - "2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.", + "2. Redistributions in binary form must reproduce the above copyright notice, \ + this list of conditions and the following disclaimer in the documentation and/or \ + other materials provided with the distribution.", ), "CC0v1": ( "CC0 1.0 Universal", - "voluntarily elects to apply CC0 to the Work and publicly distribute the Work under its terms", + "voluntarily elects to apply CC0 to the Work and publicly distribute the Work \ + under its terms", ), "GPLv2": ( "GNU General Public License version 2", @@ -206,7 +212,8 @@ class Predictor: ), "GPLv3": ( "GNU General Public License version 3", - "The GNU General Public License is a free, copyleft license for software and other kinds of works.", + "The GNU General Public License is a free, copyleft license for software and \ + other kinds of works.", ), "LGPLv2.1": ( "GNU Lesser General Public License version 2.1", @@ -218,7 +225,8 @@ class Predictor: ), "MIT": ( "The MIT License", - "including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software", + "including without limitation the rights to use, copy, modify, merge, publish, \ + distribute, sublicense, and/or sell copies of the Software", ), "MPL-2.0": ( "Mozilla Public License 2.0", @@ -227,9 +235,9 @@ class Predictor: } self._load_license() if self.license_data: - for license, test_data in known_strings.items(): - if license.lower() in self.license_data.lower(): - self.metadata.addLicense(license, self.license_file) + for shortcode, test_data in known_strings.items(): + if shortcode.lower() in self.license_data.lower(): + self.metadata.addLicense(shortcode, self.license_file) return for test_text in test_data: # Do the comparison without regard to whitespace or capitalization @@ -237,7 +245,7 @@ class Predictor: "".join(test_text.split()).lower() in "".join(self.license_data.split()).lower() ): - self.metadata.addLicense(license, self.license_file) + self.metadata.addLicense(shortcode, self.license_file) return def _predict_version(self): diff --git a/src/Mod/AddonManager/addonmanager_devmode_validators.py b/src/Mod/AddonManager/addonmanager_devmode_validators.py index aefb5e8652..ac32027f90 100644 --- a/src/Mod/AddonManager/addonmanager_devmode_validators.py +++ b/src/Mod/AddonManager/addonmanager_devmode_validators.py @@ -30,17 +30,7 @@ from PySide2.QtGui import ( ) from PySide2.QtCore import QRegularExpression - -def isidentifier(ident: str) -> bool: - - if not ident.isidentifier(): - return False - - if keyword.iskeyword(ident): - return False - - return True - +#pylint: disable=too-few-public-methods class NameValidator(QValidator): """Simple validator to exclude characters that are not valid in filenames.""" @@ -67,6 +57,7 @@ class PythonIdentifierValidator(QValidator): """Validates whether input is a valid Python identifier.""" def validate(self, value: str, _: int): + """ The function that does the validation. """ if not value: return QValidator.Intermediate