list separator in config entries

[Andreas Pakulat]

On 08.10.07 14:00:48, Thomas Braxton wrote:
> On 10/8/07, Nhuh Put <nhuh.put at web.de> wrote:
> > Please don't commit it. The QDir::(to/from)nativeSeparator stuff will
> > break on Windows
> > (for example URLs don't use \, also on Windows).
> 
> I don't understand what you mean by this, that is only used with the path
> entries (readPath/writePath/readPathList/writePathList),  but why else would
> you want to store \ in an entry unless it is a path on Windows? In that case
> using readPath/writePath should work. readEntry/writeEntry with QUrl/KUrl
> don't use the Path functions so they shouldn't be affected by this change.
> Trying to fix generic strings with \ at the end I don't think can be fixed
> without converting the \ to something else, because KConfigGroup will always
> think it is an escaped separator.

Huh? Why can't you escape the \? That works in pretty much all
programming languages including C++. Of course in that case a simple
string-replace won't work anymore, you'd have to actually parse the
string.

> Also, something like writeEntry("key", QByteArray()) will delete key, which
> > is wrong imho.
> 
> that's the point, the only times you should ever have a null entry is when
> it is first constructed before it has been set to something, and when it is
> deleted. Why else would you want to store a null QByteArray? If you look at
> the patch the reason writing an empty list failed was because I forgot to
> initialize a variable to "" and was passing a null QByteArray instead of an
> empty QByteArray. IMHO null and empty QByteArray's should _not_ be treated
> the same.

But thats wrong, see Qt docs. QByteArray makes this distinction for
historical reasons and encourages to not care about isNull() but only
for isEmpty(). 

> The main point is to get the KDE config files in sync with the freedesktop
> > spec and remove
> > the possibility for custom list separators as it will lead to too many
> > problems.
> 
> I don't have a problem with this, the only thing it will break are config
> files that stored lists/fonts/colors, but as said in another mail on this
> thread those can be upgraded
> 
> I tried kconfigtest again, and I'm still not sure the tests are valid, or
> maybe just testing something that is invalid. Since QTest stops as soon as
> it gets a failure, kconfigtest actually fails the first 2 tests, if you
> comment out the first test it will fail on the second test, then comment out
> the second test and the third test passes. So I'm still not sure how/if to
> fix this.

The only test that fails for me is testLists and thats exactly because
of the non-escaping of \ in list entries. And that testcase is correct,
there's simply no way to enforce specific contents on a configuration
entry if you allow QString. You have to make sure you handle _all_
possible strings, including backslashes. Doesn't the same problem also
appear when you put in a list like this:

("foo\,bar","barfoo")

Andreas

-- 
There was a phone call for you.