KConfig borkage
Thomas Braxton
kde.braxton at gmail.com
Mon Oct 29 02:57:30 GMT 2007
On 10/28/07, Thomas Braxton <kde.braxton at gmail.com> wrote:
>
>
>
> On 10/28/07, Oswald Buddenhagen <ossi at kde.org> wrote:
> >
> > On Sun, Oct 28, 2007 at 07:19:19AM -0400, Jeff Mitchell wrote:
> > > On Sunday 28 October 2007, Thomas Braxton wrote:
> > > > On 10/28/07, Jeff Mitchell <kde-dev at emailgoeshere.com > wrote:
> > > > > There is a [PortableDevices] section that is occasionally added to
> > > > > or deleted from. The problem is that whenever the last
> > > > > (positionally, not numerically) entry is removed, it's not
> > > > > actually removed, but rather gets corrupted somehow.
> > > >
> > > > actually is not getting corrupted, it's being marked as deleted.
> > >
> > > Why actually delete some, and only mark others?
> > >
> > if there is no default, there is no point in overshadowing it.
> > if you don't understand it, revisit the cascading config concept.
> >
> > > > Is there a default set for those keys in
> > KDEDIR/share/config/amarokrc?
> > > > because the entries shouldn't be marked as deleted unless there was
> > a
> > > > default, I'll look into this.
> > >
> > > No, there isn't. No defaults.
> > >
> > maybe a real bug then. thomas, your turn. :-P
>
>
>
> yeah a bug, I'm still trying to figure out why it's treating some entries
> as having defaults when they don't. It's in the backend when the entries are
> actually written to disk. I have a fix, but it's not quite right, it breaks
> something else, something wierd happening here. I'll try to get this fixed
> today.
>
Ok, I've had time to work on this. This patch fixes the wierdness and
removes entries from other locales from the entryMap, since I was already in
the area ;) I also added a test, since the test for "stringEntry1[fr]" was
wrong since KConfigGroup was saving the key as "stringEntry1\x5cfr\x5d", I
think those are the right codes. So take a look and let me know what you
think.
Index: tests/kconfigtest.cpp
===================================================================
--- tests/kconfigtest.cpp (revision 730047)
+++ tests/kconfigtest.cpp (working copy)
@@ -90,7 +90,6 @@
cg.writeEntry( ESCAPEKEY, ESCAPEENTRY );
cg.writeEntry( "emptyEntry", "");
cg.writeEntry( "stringEntry1", STRINGENTRY1 );
- cg.writeEntry( "stringEntry1[fr]", TRANSLATEDSTRINGENTRY1 );
cg.writeEntry( "stringEntry2", STRINGENTRY2 );
cg.writeEntry( "stringEntry3", STRINGENTRY3 );
cg.writeEntry( "stringEntry4", STRINGENTRY4 );
@@ -258,6 +257,36 @@
QCOMPARE( sc3.readEntry( "doubleEntry1", 0.0 ), DOUBLEENTRY );
}
+void KConfigTest::testLocale()
+{
+ KConfig config("kconfigtest");
+ config.setLocale("fr");
+ KConfigGroup group = config.group("Hello");
+ group.writeEntry("stringEntry1", QString(TRANSLATEDSTRINGENTRY1),
KConfig::Localized|KConfig::Persistent);
+ config.sync();
+
+ int count=0;
+ foreach(const QByteArray& line, readLines())
+ if (line.startsWith("stringEntry1[fr]"))
+ count++;
+ QVERIFY(count == 1);
+ QCOMPARE(group.readEntry("stringEntry1", QString()),
QString(TRANSLATEDSTRINGENTRY1));
+
+ config.setLocale("C"); // strings written in the "C" locale are written
as nonlocalized
+ group.writeEntry("stringEntry1", QString(STRINGENTRY1),
KConfig::Localized|KConfig::Persistent);
+ config.sync();
+
+ count=0;
+ foreach(const QByteArray& line, readLines()) {
+ if (line.startsWith("stringEntry1"))
+ count++;
+ if (line.contains("[C]"))
+ count--;
+ }
+ QVERIFY(count == 1);
+ QCOMPARE(group.readEntry("stringEntry1", QString()),
QString(STRINGENTRY1));
+}
+
void KConfigTest::testLists()
{
KConfig sc2( "kconfigtest" );
@@ -388,8 +417,6 @@
QMap<QString, QString> entryMap = cg.entryMap();
qDebug() << entryMap.keys();
QCOMPARE(entryMap.value("stringEntry1"), QString(STRINGENTRY1));
- // #### Why is this translated entry here? This just bloats the
entryMap.
- QCOMPARE(entryMap.value("stringEntry1[fr]"),
QString(TRANSLATEDSTRINGENTRY1));
QCOMPARE(entryMap.value("stringEntry2"), QString(STRINGENTRY2));
QCOMPARE(entryMap.value("stringEntry3"), QString(STRINGENTRY3));
QCOMPARE(entryMap.value("stringEntry4"), QString(STRINGENTRY4));
@@ -500,6 +527,23 @@
QVERIFY( sc.entryMap("AAA").isEmpty() );
QVERIFY( !sc.entryMap("Hello").isEmpty() ); //not deleted group
QVERIFY( sc.entryMap("FooBar").isEmpty() ); //inexistant group
+
+ // test for entries that are marked as deleted when there is no default
+ KConfig config("kconfigtest", KConfig::SimpleConfig); // make sure there
are no defaults
+ cg = config.group("Portable Devices");
+ cg.writeEntry("devices|manual|(null)", "whatever");
+ cg.writeEntry("devices|manual|/mnt/ipod", "/mnt/ipod");
+ cg.sync();
+
+ int count=0;
+ foreach(const QByteArray& item, readLines())
+ if (item.startsWith("devices|"))
+ count++;
+ QVERIFY(count == 2);
+ cg.deleteEntry("devices|manual|/mnt/ipod");
+ cg.sync();
+ foreach(const QByteArray& item, readLines())
+ QVERIFY(item != "devices|manual|/mnt/ipod[$d]");
}
void KConfigTest::testDefaultGroup()
Index: tests/kconfigtest.h
===================================================================
--- tests/kconfigtest.h (revision 730047)
+++ tests/kconfigtest.h (working copy)
@@ -36,6 +36,7 @@
void initTestCase();
void cleanupTestCase();
void testSimple();
+ void testLocale();
void testLists();
void testPath();
void testPersistenceOfExpandFlagForPath();
Index: config/kconfigdata.h
===================================================================
--- config/kconfigdata.h (revision 730047)
+++ config/kconfigdata.h (working copy)
@@ -302,6 +302,18 @@
{
return getEntryOption(findEntry(group, key, flags), option);
}
+ void removeEntry(const KEntryKey& key)
+ {
+ remove(key);
+ int count=0;
+ foreach(const KEntryKey& item, keys())
+ if(item.mGroup == key.mGroup && !item.mKey.isEmpty())
+ count++;
+ if (!count) {
+ const KEntryKey groupKey(key.mGroup);
+ remove(groupKey);
+ }
+ }
void setEntryOption(Iterator it, EntryOption option, bool bf)
{
Index: config/kconfigini.cpp
===================================================================
--- config/kconfigini.cpp (revision 730047)
+++ config/kconfigini.cpp (working copy)
@@ -202,10 +202,8 @@
if (!locale.isEmpty()) {
if (locale != currentLocale) {
// backward compatibility. C == en_US
- if (locale.at(0) != 'C' || currentLocale != "en_US") {
- aKey += '[' + locale + ']';
- locale = QByteArray();
- }
+ if (locale.at(0) != 'C' || currentLocale != "en_US")
+ goto next_line;
}
}
@@ -234,28 +232,11 @@
if ((key.mGroup != "<default>") == defaultGroup)
continue; // skip
- // Skip default values and group headers
- if (key.bDefault || key.mKey.isNull())
+ // Skip group headers
+ if (key.mKey.isNull())
continue; // skip
const KEntry& currentEntry = *it;
- KEntryMapConstIterator testIt = it;
- ++testIt;
- if (testIt != end) { // might have a default
- const KEntryKey& defaultKey = testIt.key();
- if (defaultKey.bDefault ||
- defaultKey.mGroup != key.mGroup ||
- defaultKey.mKey != key.mKey ||
- defaultKey.bLocal != key.bLocal) {
- if (currentEntry.bDeleted)
- continue; // Don't write deleted entries if there is no
default
- } else {
- if (currentEntry.mValue == testIt->mValue &&
- currentEntry.bDeleted == testIt->bDeleted)
- continue; // same as default, don't write
- }
- }
-
if (!defaultGroup && currentGroup != key.mGroup) {
if (!firstEntry)
file.putChar('\n');
@@ -270,7 +251,7 @@
file.write(stringToPrintable(key.mKey)); // Key
- if (currentEntry.bNLS) {
+ if (currentEntry.bNLS && locale != "C") {
file.putChar('[');
file.write(locale); // Locale tag
file.putChar(']');
@@ -298,29 +279,37 @@
{
Q_ASSERT(!filePath().isEmpty());
- KEntryMap tempMap = toMerge;
+ KEntryMap writeMap = toMerge;
bool bGlobal = options & WriteGlobal;
- int mergeCount = toMerge.count();
const KEntryMapIterator end = entryMap.end();
for (KEntryMapIterator it=entryMap.begin(); it != end; ++it) {
- const KEntryKey& key = it.key();
- if (key.bDefault) {
- tempMap[key] = *it; // store default values in tempMap
+ if (!it->bDirty) // not dirty, doesn't overwrite entry in writeMap
continue;
- }
- if (mergeCount > 0 && !it->bDirty)
- continue;
- // only write back dirty entries that have the same
- // "globality" as the file
- if (it->bGlobal == bGlobal && it->bDirty) {
- tempMap[key] = *it;
+ const KEntryKey& key = it.key();
+
+ // only write back dirty entries that have the same "globality" as
the file
+ if (it->bGlobal == bGlobal) {
+ KEntryKey defaultKey = key;
+ defaultKey.bDefault = true;
+ if (it->bDeleted && !entryMap.contains(defaultKey))
+ writeMap.removeEntry(key); // remove the deleted entry if
there is no default
+ else
+ writeMap[key] = *it;
+
it->bDirty = false;
}
}
- // now tempMap should contain all entries to be written
+ if (writeMap.isEmpty()) {
+ QFile file(filePath());
+ if (file.exists())
+ file.remove(); // delete the file since it's going to be empty
anyways
+ return true;
+ }
+
+ // now writeMap should contain only entries to be written
// so write it out to disk
// check if file exists
@@ -352,8 +341,9 @@
if (createNew)
file.setPermissions(QFile::ReadUser|QFile::WriteUser);
- writeEntries(locale, file, tempMap);
+ writeEntries(locale, file, writeMap);
+ // FIXME is this needed anymore?
if ( !file.size() && ((fileMode == -1) || (fileMode == 0600)) ) {
// File is empty and doesn't have special permissions: delete it.
file.abort();
@@ -432,7 +422,6 @@
if (lockFile->lock() == KLockFile::LockStale) // attempt to break the
lock
lockFile->lock(KLockFile::ForceFlag);
return lockFile->isLocked();
- return true;
}
void KConfigIniBackend::unlock()
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20071028/1922ec20/attachment.htm>
More information about the kde-core-devel
mailing list