[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: [Panel-devel] [PATCH] Package and PackageStructure cleanup
From: Paolo Capriotti <p.capriotti () gmail ! com>
Date: 2007-11-25 0:01:52
Message-ID: fiae1i$e8n$1 () ger ! gmane ! org
[Download RAW message or body]
Hi,
while working on background selection with ruphy, I've found some
oddities in the existing Package code. Here is a cleanup patch that:
1) replaces all const char* with QString in the API;
2) changes the Package constructor to a single path argument;
3) makes some methods in PackageStructure const;
4) updates the only usage of Package I could find, and unit tests.
Ok to commit?
While we are at it, I was wondering if a more fundamental refactoring of
the Package classes could be useful. Since there is almost no code using
them, I would like to know what the use cases are, so that we can think
about improving the API and functionality in the next few days.
Paolo Capriotti
["cleanup2.patch" (text/x-diff)]
Index: package.h
===================================================================
--- package.h (revision 741140)
+++ package.h (working copy)
@@ -45,8 +45,7 @@
* @arg package the name of the package
* @arg structure the package structure describing this package
**/
- Package(const QString& packageRoot, const QString& package,
- const PackageStructure& structure);
+ Package(const QString& package, const PackageStructure& structure);
~Package();
/**
@@ -63,7 +62,7 @@
* @arg filename the name of the file
* @return path to the file on disk. QString() if not found.
**/
- QString filePath(const char* fileType, const QString& filename) const;
+ QString filePath(const QString &fileType, const QString& filename) const;
/**
* Get the path to a given file.
@@ -73,7 +72,7 @@
* in the package structure and not a directory.
* @return path to the file on disk. QString() if not found
**/
- QString filePath(const char* fileType) const;
+ QString filePath(const QString &fileType) const;
/**
* Get the list of files of a given type.
@@ -82,7 +81,7 @@
* package structure.
* @return list of files by name, suitable for passing to filePath
**/
- QStringList entryList(const char* fileType) const;
+ QStringList entryList(const QString &fileType) const;
/**
* Returns a list of all installed packages
Index: package.cpp
===================================================================
--- package.cpp (revision 741140)
+++ package.cpp (working copy)
@@ -62,9 +62,9 @@
bool valid;
};
-Package::Package(const QString& packageRoot, const QString& package,
+Package::Package(const QString& package,
const PackageStructure& structure)
- : d(new Private(structure, packageRoot + '/' + package))
+ : d(new Private(structure, package))
{
}
@@ -79,14 +79,14 @@
return false;
}
- foreach (const char *dir, d->structure.requiredDirectories()) {
+ foreach (QString dir, d->structure.requiredDirectories()) {
if (!QFile::exists(d->basePath + d->structure.path(dir))) {
d->valid = false;
return false;
}
}
- foreach (const char *file, d->structure.requiredFiles()) {
+ foreach (QString file, d->structure.requiredFiles()) {
if (!QFile::exists(d->basePath + d->structure.path(file))) {
d->valid = false;
return false;
@@ -96,7 +96,7 @@
return true;
}
-QString Package::filePath(const char* fileType, const QString& filename) const
+QString Package::filePath(const QString &fileType, const QString& filename) const
{
if (!d->valid) {
return QString();
@@ -121,12 +121,12 @@
return QString();
}
-QString Package::filePath(const char* fileType) const
+QString Package::filePath(const QString &fileType) const
{
return filePath(fileType, QString());
}
-QStringList Package::entryList(const char* fileType) const
+QStringList Package::entryList(const QString &fileType) const
{
if (!d->valid) {
return QStringList();
Index: packagestructure.cpp
===================================================================
--- packagestructure.cpp (revision 741140)
+++ packagestructure.cpp (working copy)
@@ -53,7 +53,7 @@
{
public:
QString type;
- QHash<const char*, ContentStructure> contents;
+ QHash<QString, ContentStructure> contents;
QStringList mimetypes;
};
@@ -84,15 +84,15 @@
return *this;
}
-QString PackageStructure::type()
+QString PackageStructure::type() const
{
return d->type;
}
-QList<const char*> PackageStructure::directories()
+QStringList PackageStructure::directories() const
{
- QList<const char*> dirs;
- QHash<const char*, ContentStructure>::const_iterator it = \
d->contents.constBegin(); + QStringList dirs;
+ QHash<QString, ContentStructure>::const_iterator it = d->contents.constBegin();
while (it != d->contents.constEnd()) {
if (it.value().directory) {
dirs << it.key();
@@ -102,10 +102,10 @@
return dirs;
}
-QList<const char*> PackageStructure::requiredDirectories()
+QStringList PackageStructure::requiredDirectories() const
{
- QList<const char*> dirs;
- QHash<const char*, ContentStructure>::const_iterator it = \
d->contents.constBegin(); + QStringList dirs;
+ QHash<QString, ContentStructure>::const_iterator it = d->contents.constBegin();
while (it != d->contents.constEnd()) {
if (it.value().directory &&
it.value().required) {
@@ -116,10 +116,10 @@
return dirs;
}
-QList<const char*> PackageStructure::files()
+QStringList PackageStructure::files() const
{
- QList<const char*> files;
- QHash<const char*, ContentStructure>::const_iterator it = \
d->contents.constBegin(); + QStringList files;
+ QHash<QString, ContentStructure>::const_iterator it = d->contents.constBegin();
while (it != d->contents.constEnd()) {
if (!it.value().directory) {
files << it.key();
@@ -129,10 +129,10 @@
return files;
}
-QList<const char*> PackageStructure::requiredFiles()
+QStringList PackageStructure::requiredFiles() const
{
- QList<const char*> files;
- QHash<const char*, ContentStructure>::const_iterator it = \
d->contents.constBegin(); + QStringList files;
+ QHash<QString, ContentStructure>::const_iterator it = d->contents.constBegin();
while (it != d->contents.constEnd()) {
if (!it.value().directory && it.value().required) {
files << it.key();
@@ -142,7 +142,7 @@
return files;
}
-void PackageStructure::addDirectoryDefinition(const char* key, const QString& path, \
const QString& name) +void PackageStructure::addDirectoryDefinition(const QString \
&key, const QString& path, const QString& name) {
ContentStructure s;
s.name = name;
@@ -152,7 +152,7 @@
d->contents[key] = s;
}
-void PackageStructure::addFileDefinition(const char* key, const QString& path, const \
QString& name) +void PackageStructure::addFileDefinition(const QString &key, const \
QString& path, const QString& name) {
ContentStructure s;
s.name = name;
@@ -162,9 +162,9 @@
d->contents[key] = s;
}
-QString PackageStructure::path(const char* key)
+QString PackageStructure::path(const QString &key) const
{
- QHash<const char*, ContentStructure>::const_iterator it = d->contents.find(key);
+ QHash<QString, ContentStructure>::const_iterator it = d->contents.find(key);
if (it == d->contents.constEnd()) {
return QString();
}
@@ -172,9 +172,9 @@
return it.value().path;
}
-QString PackageStructure::name(const char* key)
+QString PackageStructure::name(const QString &key) const
{
- QHash<const char*, ContentStructure>::const_iterator it = d->contents.find(key);
+ QHash<QString, ContentStructure>::const_iterator it = d->contents.find(key);
if (it == d->contents.constEnd()) {
return QString();
}
@@ -182,9 +182,9 @@
return it.value().name;
}
-void PackageStructure::setRequired(const char* key, bool required)
+void PackageStructure::setRequired(const QString &key, bool required)
{
- QHash<const char*, ContentStructure>::iterator it = d->contents.find(key);
+ QHash<QString, ContentStructure>::iterator it = d->contents.find(key);
if (it == d->contents.end()) {
return;
}
@@ -192,9 +192,9 @@
it.value().required = required;
}
-bool PackageStructure::required(const char* key)
+bool PackageStructure::required(const QString &key) const
{
- QHash<const char*, ContentStructure>::const_iterator it = d->contents.find(key);
+ QHash<QString, ContentStructure>::const_iterator it = d->contents.find(key);
if (it == d->contents.constEnd()) {
return false;
}
@@ -207,9 +207,9 @@
d->mimetypes = mimetypes;
}
-void PackageStructure::setMimetypes(const char* key, QStringList mimetypes)
+void PackageStructure::setMimetypes(const QString &key, QStringList mimetypes)
{
- QHash<const char*, ContentStructure>::iterator it = d->contents.find(key);
+ QHash<QString, ContentStructure>::iterator it = d->contents.find(key);
if (it == d->contents.end()) {
return;
}
@@ -217,9 +217,9 @@
it.value().mimetypes = mimetypes;
}
-QStringList PackageStructure::mimetypes(const char* key)
+QStringList PackageStructure::mimetypes(const QString &key) const
{
- QHash<const char*, ContentStructure>::const_iterator it = d->contents.find(key);
+ QHash<QString, ContentStructure>::const_iterator it = d->contents.find(key);
if (it == d->contents.constEnd()) {
return QStringList();
}
Index: applet.cpp
===================================================================
--- applet.cpp (revision 741140)
+++ applet.cpp (working copy)
@@ -134,9 +134,7 @@
if (!path.isEmpty()) {
// create the package and see if we have something real
- package = new Package(path,
- appletDescription.pluginName(),
- PlasmoidStructure());
+ package = new Package(path, PlasmoidStructure());
if (package->isValid()) {
// now we try and set up the script engine.
// it will be parented to this applet and so will get
Index: packagestructure.h
===================================================================
--- packagestructure.h (revision 741140)
+++ packagestructure.h (working copy)
@@ -70,27 +70,27 @@
/**
* Type of package this structure describes
**/
- QString type();
+ QString type() const;
/**
* The directories defined for this package
**/
- QList<const char*> directories();
+ QStringList directories() const;
/**
* The required directories defined for this package
**/
- QList<const char*> requiredDirectories();
+ QStringList requiredDirectories() const;
/**
* The individual files, by key, that are defined for this package
**/
- QList<const char*> files();
+ QStringList files() const;
/**
* The individual required files, by key, that are defined for this package
**/
- QList<const char*> requiredFiles();
+ QStringList requiredFiles() const;
/**
* Adds a directory to the structure of the package. It is added as
@@ -100,7 +100,7 @@
* @param path the path within the the package for this directory
* @param name the user visible (translated) name for the directory
**/
- void addDirectoryDefinition(const char* key, const QString& path, const QString& \
name); + void addDirectoryDefinition(const QString &key, const QString& path, \
const QString& name);
/**
* Adds a file to the structure of the package. It is added as
@@ -110,17 +110,17 @@
* @param path the path within the the package for this file
* @param name the user visible (translated) name for the file
**/
- void addFileDefinition(const char* key, const QString& path, const QString& \
name); + void addFileDefinition(const QString &key, const QString& path, const \
QString& name);
/**
* @return path relative to the package root for the given entry
**/
- QString path(const char* key);
+ QString path(const QString &key) const;
/**
* @return user visible name for the given entry
**/
- QString name(const char* key);
+ QString name(const QString &key) const;
/**
* Sets whether or not a given part of the structure is required or not.
@@ -130,12 +130,12 @@
* @param path the path of the entry within the package
* @param required true if this entry is required, false if not
*/
- void setRequired(const char* key, bool required);
+ void setRequired(const QString &key, bool required);
/**
* @return true if the item at path exists and is required
**/
- bool required(const char* key);
+ bool required(const QString &key) const;
/**
* Defines the default mimetypes for any definitions that do not have
@@ -154,12 +154,12 @@
* @param path the path of the entry within the package
* @param mimetypes a list of mimetypes
**/
- void setMimetypes(const char* key, QStringList mimetypes);
+ void setMimetypes(const QString &key, QStringList mimetypes);
/**
* @return the mimetypes associated with the path, if any
**/
- QStringList mimetypes(const char* key);
+ QStringList mimetypes(const QString &key) const;
/**
* Copy constructor
Index: tests/plasmoidpackagetest.cpp
===================================================================
--- tests/plasmoidpackagetest.cpp (revision 741140)
+++ tests/plasmoidpackagetest.cpp (working copy)
@@ -26,8 +26,8 @@
void PlasmoidPackageTest::init()
{
- mPackage = QString("Package");
- mPackageRoot = QDir::homePath() + "/.kde-unit-test/packageRoot";
+ mPackageRoot = QDir::homePath() + "/.kde-unit-test/packageRoot/";
+ mPackage = mPackageRoot + "Package";
ps = new Plasma::PlasmoidStructure;
}
@@ -127,7 +127,7 @@
void PlasmoidPackageTest::isValid()
{
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
// A PlasmoidPackage is valid when:
// - The package root exists.
@@ -140,11 +140,11 @@
// Should still be invalid.
delete p;
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
QVERIFY(!p->isValid());
// Create the metadata.desktop file.
- QFile file(mPackageRoot + "/" + mPackage + "/metadata.desktop");
+ QFile file(mPackage + "/metadata.desktop");
QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text));
QTextStream out(&file);
@@ -153,15 +153,15 @@
file.close();
// Create the code dir.
- QVERIFY(QDir().mkpath(mPackageRoot + "/" + mPackage + "/code"));
+ QVERIFY(QDir().mkpath(mPackage + "/code"));
// No main file yet so should still be invalid.
delete p;
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
QVERIFY(!p->isValid());
// Create the main file.
- file.setFileName(mPackageRoot + "/" + mPackage + "/code/main");
+ file.setFileName(mPackage + "/code/main");
QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text));
out.setDevice(&file);
@@ -171,7 +171,7 @@
// Main file exists so should be valid now.
delete p;
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
QVERIFY(p->isValid());
}
@@ -180,12 +180,12 @@
// Package::filePath() returns
// - {package_root}/{package_name}/path/to/file if the file exists
// - QString() otherwise.
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
QCOMPARE(p->filePath("scripts", "main"), QString());
- QVERIFY(QDir().mkpath(mPackageRoot + "/" + mPackage + "/code"));
- QFile file(mPackageRoot + "/" + mPackage + "/code/main");
+ QVERIFY(QDir().mkpath(mPackage + "/code"));
+ QFile file(mPackage + "/code/main");
QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text));
QTextStream out(&file);
@@ -195,9 +195,9 @@
// The package is valid by now so a path for code/main should get returned.
delete p;
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
- QString path = mPackageRoot + "/" + mPackage + "/code/main";
+ QString path = mPackage + "/code/main";
// Two ways to get the same info.
// 1. Give the file type which refers to a class of files (a directory) in
@@ -218,7 +218,7 @@
createTestPackage(packageName);
// Create a package object and verify that it is valid.
- p = new Plasma::Package(mPackageRoot, packageName, *ps);
+ p = new Plasma::Package(mPackageRoot + packageName, *ps);
QVERIFY(p->isValid());
// Now we have a valid package that should contain the following files in
@@ -241,27 +241,27 @@
// Don't do strange things when package root doesn't exists.
QDir pRoot = QDir(mPackageRoot);
QVERIFY(!pRoot.exists());
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
QCOMPARE(p->knownPackages(mPackageRoot), QStringList());
delete p;
// Don't do strange things when an empty package root exists
QVERIFY(QDir().mkpath(mPackageRoot));
//QVERIFY(pRoot.exists());
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
QCOMPARE(p->knownPackages(mPackageRoot), QStringList());
delete p;
// Do not return a directory as package if it has no metadata.desktop file
QVERIFY(QDir().mkpath(mPackageRoot + "/invalid_plasmoid"));
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
QCOMPARE(p->knownPackages(mPackageRoot), QStringList());
delete p;
// Let's add a valid package and see what happens.
QString plamoid1("a_valid_plasmoid");
createTestPackage(plamoid1);
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
QStringList packages = p->knownPackages(mPackageRoot);
QCOMPARE(packages.size(), 1);
@@ -270,7 +270,7 @@
// Ok.... one more valid package.
QString plamoid2("anoter_valid_plasmoid");
createTestPackage(plamoid2);
- p = new Plasma::Package(mPackageRoot, mPackage, *ps);
+ p = new Plasma::Package(mPackage, *ps);
packages = p->knownPackages(mPackageRoot);
QCOMPARE(packages.size(), 2);
Index: tests/packagestructuretest.cpp
===================================================================
--- tests/packagestructuretest.cpp (revision 741140)
+++ tests/packagestructuretest.cpp (working copy)
@@ -38,11 +38,11 @@
#include <KDebug>
void PackageStructureTest::directories()
{
- QList<const char*> dirs;
+ QStringList dirs;
dirs << "images" << "config" << "configui" << "scripts";
qSort(dirs);
- QList<const char*> psDirs = ps->directories();
+ QStringList psDirs = ps->directories();
qSort(psDirs);
QCOMPARE(psDirs, dirs);
@@ -50,17 +50,17 @@
void PackageStructureTest::requiredDirectories()
{
- QList<const char*> dirs;
+ QStringList dirs;
QCOMPARE(ps->requiredDirectories(), dirs);
}
void PackageStructureTest::files()
{
- QList<const char*> files;
+ QStringList files;
files << "mainconfiggui" << "mainconfigxml" << "mainscript";
qSort(files);
- QList<const char*> psFiles = ps->files();
+ QStringList psFiles = ps->files();
qSort(psFiles);
QCOMPARE(psFiles, files);
@@ -68,7 +68,7 @@
void PackageStructureTest::requiredFiles()
{
- QList<const char*> files;
+ QStringList files;
files << "mainscript";
QCOMPARE(ps->requiredFiles(), files);
}
_______________________________________________
Panel-devel mailing list
Panel-devel@kde.org
https://mail.kde.org/mailman/listinfo/panel-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic