[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: KDE/kdelibs/kdecore
From: David Faure <faure () kde ! org>
Date: 2010-12-09 16:39:47
Message-ID: 20101209163947.979A0AC8A5 () svn ! kde ! org
[Download RAW message or body]
SVN commit 1204914 by dfaure:
Fix deletion of groups so that a group doesn't exist() after being deleted :-)
A group now exists if it has at least one non-deleted entry (including in subgroups).
+ added unittest for deleting a nested group, and check what gets written to disk.
BUG: 192266
FIXED-IN: 4.6
M +54 -32 config/kconfig.cpp
M +3 -0 config/kconfig_p.h
M +10 -6 config/kconfigdata.h
M +8 -3 config/kconfigini.cpp
M +32 -2 tests/kconfigtest.cpp
--- trunk/KDE/kdelibs/kdecore/config/kconfig.cpp #1204913:1204914
@@ -207,9 +207,10 @@
QSet<QString> groups;
for (KEntryMap::ConstIterator entryMapIt( d->entryMap.constBegin() ); entryMapIt \
!= d->entryMap.constEnd(); ++entryMapIt) {
- const QByteArray group = entryMapIt.key().mGroup;
- if (entryMapIt.key().mKey.isNull() && !group.isEmpty() && group != \
"<default>" && group != "$Version") {
- QString groupname = QString::fromUtf8(group);
+ const KEntryKey& key = entryMapIt.key();
+ const QByteArray group = key.mGroup;
+ if (key.mKey.isNull() && !group.isEmpty() && group != "<default>" && group \
!= "$Version") { + const QString groupname = QString::fromUtf8(group);
groups << groupname.left(groupname.indexOf(QLatin1Char('\x1d')));
}
}
@@ -222,23 +223,53 @@
QByteArray theGroup = group + '\x1d';
QSet<QString> groups;
- for (KEntryMap::ConstIterator entryMapIt( entryMap.constBegin() ); entryMapIt != \
entryMap.constEnd(); ++entryMapIt)
- if (entryMapIt.key().mKey.isNull() && \
entryMapIt.key().mGroup.startsWith(theGroup)) {
- const QString groupname = \
QString::fromUtf8(entryMapIt.key().mGroup.mid(theGroup.length())); + for \
(KEntryMap::ConstIterator entryMapIt( entryMap.constBegin() ); entryMapIt != \
entryMap.constEnd(); ++entryMapIt) { + const KEntryKey& key = \
entryMapIt.key(); + if (key.mKey.isNull() && key.mGroup.startsWith(theGroup)) \
{ + const QString groupname = \
QString::fromUtf8(key.mGroup.mid(theGroup.length()));
groups << groupname.left(groupname.indexOf(QLatin1Char('\x1d')));
}
+ }
return groups.toList();
}
-QStringList KConfig::keyList(const QString& aGroup) const
+// List all sub groups, including subsubgroups
+QSet<QByteArray> KConfigPrivate::allSubGroups(const QByteArray& parentGroup) const
{
- Q_D(const KConfig);
+ QSet<QByteArray> groups;
+ QByteArray theGroup = parentGroup + '\x1d';
+ groups << parentGroup;
+
+ for (KEntryMap::const_iterator entryMapIt = entryMap.begin(); entryMapIt != \
entryMap.end(); ++entryMapIt) { + const KEntryKey& key = entryMapIt.key();
+ if (key.mKey.isNull() && key.mGroup.startsWith(theGroup)) {
+ groups << key.mGroup;
+ }
+ }
+ return groups;
+}
+
+bool KConfigPrivate::hasNonDeletedEntries(const QByteArray& group) const
+{
+ const QSet<QByteArray> allGroups = allSubGroups(group);
+
+ Q_FOREACH(const QByteArray& aGroup, allGroups) {
+ // Could be optimized, let's use the slow way for now
+ // Check for any non-deleted entry
+ if (!keyListImpl(aGroup).isEmpty())
+ return true;
+ }
+ return false;
+}
+
+
+QStringList KConfigPrivate::keyListImpl(const QByteArray& theGroup) const
+{
QStringList keys;
- const QByteArray theGroup(aGroup.isEmpty() ? "<default>" : aGroup.toUtf8());
- const KEntryMapConstIterator theEnd = d->entryMap.constEnd();
- KEntryMapConstIterator it = d->entryMap.findEntry(theGroup);
+ const KEntryMapConstIterator theEnd = entryMap.constEnd();
+ KEntryMapConstIterator it = entryMap.findEntry(theGroup);
if (it != theEnd) {
++it; // advance past the special group entry marker
@@ -254,6 +285,13 @@
return keys;
}
+QStringList KConfig::keyList(const QString& aGroup) const
+{
+ Q_D(const KConfig);
+ const QByteArray theGroup(aGroup.isEmpty() ? "<default>" : aGroup.toUtf8());
+ return d->keyListImpl(theGroup);
+}
+
QMap<QString,QString> KConfig::entryMap(const QString& aGroup) const
{
Q_D(const KConfig);
@@ -657,18 +695,9 @@
Q_D(KConfig);
KEntryMap::EntryOptions options = \
convertToOptions(flags)|KEntryMap::EntryDeleted;
- QByteArray theGroup = aGroup + '\x1d';
- QSet<QByteArray> groups;
- groups << aGroup;
-
- for (KEntryMap::ConstIterator entryMapIt( d->entryMap.constBegin() ); entryMapIt \
!= d->entryMap.constEnd(); ++entryMapIt) {
- if (entryMapIt.key().mKey.isNull() && \
entryMapIt.key().mGroup.startsWith(theGroup)) {
- groups << entryMapIt.key().mGroup;
- }
- }
-
+ const QSet<QByteArray> groups = d->allSubGroups(aGroup);
foreach (const QByteArray& group, groups) {
- const QStringList keys = keyList(QString::fromUtf8(group));
+ const QStringList keys = d->keyListImpl(group);
foreach (const QString& _key, keys) {
const QByteArray &key = _key.toUtf8();
if (d->canWriteEntry(group, key.constData())) {
@@ -709,18 +738,11 @@
{
Q_D(const KConfig);
- if (d->entryMap.hasEntry(aGroup)) return true;
+ // No need to look for the actual group entry anymore, or for subgroups:
+ // a group exists if it contains any non-deleted entry.
- QByteArray subGroupMarker = aGroup + '\x1d';
- // Because this group could have only subgroups but no entries we have to
- // search for group markers
- for (KEntryMap::ConstIterator entryMapIt( d->entryMap.constBegin() ); entryMapIt \
!= d->entryMap.constEnd(); ++entryMapIt) {
- if (entryMapIt.key().mGroup.startsWith(subGroupMarker)) {
- return true;
+ return d->hasNonDeletedEntries(aGroup);
}
- }
- return false;
-}
bool KConfigPrivate::canWriteEntry(const QByteArray& group, const char* key, bool \
isDefault) const {
--- trunk/KDE/kdelibs/kdecore/config/kconfig_p.h #1204913:1204914
@@ -61,6 +61,9 @@
// of @p source with @p destination
void copyGroup(const QByteArray& source, const QByteArray& destination,
KConfigGroup *otherGroup, KConfigBase::WriteConfigFlags flags) \
const; + QStringList keyListImpl(const QByteArray& theGroup) const;
+ QSet<QByteArray> allSubGroups(const QByteArray& parentGroup) const;
+ bool hasNonDeletedEntries(const QByteArray& group) const;
protected:
KSharedPtr<KConfigBackend> mBackend;
--- trunk/KDE/kdelibs/kdecore/config/kconfigdata.h #1204913:1204914
@@ -226,12 +226,13 @@
if (key.isEmpty()) { // inserting a group marker
k.mGroup = group;
e.bImmutable = (options&EntryImmutable);
- if(it == end())
- {
+ if (options&EntryDeleted) { qWarning("Internal KConfig error: cannot \
mark groups as deleted"); } + if(it == end()) {
insert(k, e);
return true;
- } else if(it.value() == e)
+ } else if(it.value() == e) {
return false;
+ }
it.value() = e;
return true;
@@ -274,7 +275,7 @@
e.bDeleted = false; // setting a value to a previously deleted entry
e.bExpand = (options&EntryExpansion);
- //qDebug() << "to [" << group << "," << key << "] =" << e.mValue;
+ //qDebug() << "to [" << group << "," << key << "] =" << e.mValue << \
"newKey=" << newKey; if(newKey)
{
insert(k, e);
@@ -365,10 +366,13 @@
const ConstIterator it = findEntry(group, key, flags);
if (it == constEnd())
return false;
- if (key.isNull()) // looking for group marker
+ if (it->bDeleted)
+ return false;
+ if (key.isNull()) { // looking for group marker
return it->mValue.isNull();
- return !it->bDeleted;
}
+ return true;
+ }
bool getEntryOption(const ConstIterator& it, EntryOption option) const
{
--- trunk/KDE/kdelibs/kdecore/config/kconfigini.cpp #1204913:1204914
@@ -368,8 +368,9 @@
Q_ASSERT(!filePath().isEmpty());
KEntryMap writeMap;
- bool bGlobal = options & WriteGlobal;
+ const bool bGlobal = options & WriteGlobal;
+ // First, reparse the file on disk, to merge our changes with the ones done by \
other apps {
ParseOptions opts = ParseExpansions;
if (bGlobal)
@@ -378,6 +379,7 @@
if (info != ParseOk) // either there was an error or the file became \
immutable return false;
}
+
const KEntryMapIterator end = entryMap.end();
for (KEntryMapIterator it=entryMap.begin(); it != end; ++it) {
if (!it.key().mKey.isEmpty() && !it->bDirty) // not dirty, doesn't overwrite \
entry in writeMap. skips default entries, too. @@ -392,11 +394,14 @@
} else {
KEntryKey defaultKey = key;
defaultKey.bDefault = true;
- if (!entryMap.contains(defaultKey))
+ if (!entryMap.contains(defaultKey)) {
writeMap.remove(key); // remove the deleted entry if there is no \
default
- else
+ //qDebug() << "Detected as deleted=>removed:" << key.mGroup << \
key.mKey << "global=" << bGlobal; + } else {
writeMap[key] = *it; // otherwise write an explicitly deleted \
entry + //qDebug() << "Detected as deleted=>[$d]:" << key.mGroup \
<< key.mKey << "global=" << bGlobal; }
+ }
it->bDirty = false;
}
}
--- trunk/KDE/kdelibs/kdecore/tests/kconfigtest.cpp #1204913:1204914
@@ -133,6 +133,9 @@
cg = KConfigGroup(&cg, "Nested Group 2.1");
cg.writeEntry( "stringEntry3", STRINGENTRY3 );
+ cg = KConfigGroup(&ct, "Nested Group 3");
+ cg.writeEntry( "stringEntry3", STRINGENTRY3 );
+
cg = KConfigGroup(&sc, "List Types" );
cg.writeEntry( "listOfIntsEntry1", INTLISTENTRY1 );
cg.writeEntry( "listOfByteArraysEntry1", BYTEARRAYLISTENTRY1 );
@@ -667,10 +670,25 @@
KConfig sc( "kconfigtest" );
KConfigGroup ct(&sc, "Complex Types");
+
+ // First delete a nested group
+ KConfigGroup delgr(&ct, "Nested Group 3");
+ QVERIFY(delgr.exists());
+ QVERIFY(ct.hasGroup("Nested Group 3"));
+ delgr.deleteGroup();
+ QVERIFY(!delgr.exists());
+ QVERIFY(!ct.hasGroup("Nested Group 3"));
+ QVERIFY(ct.groupList().contains("Nested Group 3"));
+
KConfigGroup ng(&ct, "Nested Group 2");
+ QVERIFY(sc.hasGroup("Complex Types"));
+ QVERIFY(!sc.hasGroup("Does not exist"));
sc.deleteGroup("Complex Types");
QCOMPARE(sc.group("Complex Types").keyList().count(), 0);
- QVERIFY(sc.group("Complex Types").exists()); // yep, we deleted it, but it still \
"exists"... + QVERIFY(!sc.hasGroup("Complex Types")); // #192266
+ QVERIFY(!sc.group("Complex Types").exists());
+ QVERIFY(!ct.hasGroup("Nested Group 1"));
+
QCOMPARE(ct.group("Nested Group 1").keyList().count(), 0);
QCOMPARE(ct.group("Nested Group 2").keyList().count(), 0);
QCOMPARE(ng.group("Nested Group 2.1").keyList().count(), 0);
@@ -682,6 +700,17 @@
QVERIFY( !sc.entryMap("Hello").isEmpty() ); //not deleted group
QVERIFY( sc.entryMap("FooBar").isEmpty() ); //inexistant group
+ cg.sync();
+ // Check what happens on disk
+ const QList<QByteArray> lines = readLines();
+ //qDebug() << lines;
+ QVERIFY(!lines.contains("[Complex Types]\n"));
+ QVERIFY(!lines.contains("[Complex Types][Nested Group 1]\n"));
+ QVERIFY(!lines.contains("[Complex Types][Nested Group 2]\n"));
+ QVERIFY(!lines.contains("[Complex Types][Nested Group 2.1]\n"));
+ QVERIFY(!lines.contains("[AAA]\n"));
+ QVERIFY(lines.contains("[Hello]\n")); // a group that was not deleted
+
// test for entries that are marked as deleted when there is no default
KConfig cf("kconfigtest", KConfig::SimpleConfig); // make sure there are no \
defaults cg = cf.group("Portable Devices");
@@ -1394,10 +1423,11 @@
// Current state: [ca] and [de] entries left... oops.
//qDebug() << readLinesFrom(file);
- // The group still exists but the values are all gone.
+ // Bug: The group still exists [because of the localized entries]...
QVERIFY(cg.exists());
QVERIFY(!cg.hasKey("foo"));
QVERIFY(!cg.hasKey("foostring"));
+ QEXPECT_FAIL("", "Currently localized values are not deleted correctly", \
Continue); QVERIFY(!cg.hasKey("foobool"));
// Now switch the locale to "de" and repeat the checks. All values
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic