list separator in config entries

[Thomas Braxton]

On 10/8/07, Nhuh Put <nhuh.put at web.de> wrote:
>
> 2007/10/8, Thomas Braxton <kde.braxton at gmail.com>:
> >
> >
> > On 10/8/07, Andreas Hartmetz <ahartmetz at gmail.com> wrote:
> > > 2007/10/8, Thomas Braxton <kde.braxton at gmail.com>:
> > > >
> > > >
> > > >
> > > > On 10/8/07, Thomas Braxton <kde.braxton at gmail.com > wrote:
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On 10/8/07, Oswald Buddenhagen < ossi at kde.org> wrote:
> > > > > > On Mon, Oct 08, 2007 at 01:22:02PM +0200, Andreas Hartmetz
> wrote:
> > > > > > > 2007/10/8, Oswald Buddenhagen <ossi at kde.org >:
> > > > > > > > the question is at which point lists are serialized and
> > therefore
> > > > > > > > through how many escaping passes they go.
> > > > > > > >
> > > > > > > Ahem yes, I missed that.
> > > > > > > You are arguing for another refactoring of KConfig here which
> I
> > assume
> > > > > > > was not your intention.
> > > > > > >
> > > > > > whoops. ;)
> > > > > >
> > > > > > > I agree that we need to fix things right,
> > > > > > >
> > > > > > exactly.
> > > > > > if another refactoring is the price to pay, we need to bite the
> > bullet.
> > > > > > note that this is only internal stuff, so it doesn't necessarily
> > mean a
> > > > > > further delay.
> > > > > >
> > > > > > > For the record: The refactoring would consist of moving list
> > handling
> > > > > > > into the backend(s)
> > > > > > >
> > > > > > yes.
> > > > > > given that the windows registry can handle numerical values and
> > blobs,
> > > > > > too, all of the type handling should move to the backends - the
> api
> > > > > > would only know qvariant, which would allow arbitrarily complex
> > nesting
> > > > > > and leave complete freedom to the backend.
> > > > > > to support backends that don't provide arbitrary binary storage
> (in
> > > > > > particular, text-oriented backends like ini), a serialization
> > > > > > class/namespace would be provided to share code.
> > > > > > serializers for gui classes need to be somehow external ...
> hmmm,
> > > > > > actually, this is a generic problem if custom data structures
> should
> > be
> > > > > > supported.
> > > > > >
> > > > > > > > assuming that the spec is implemented only by people who
> know
> > > > > > > > anything about working escaping is pretty naive.
> > > > > > > >
> > > > > > > Programmers OTOH should know what they are doing.
> > > > > > >
> > > > > > the key is "should". particularly in the lesser known
> almost-one-man
> > > > > > projects the know-how is often missing. it's incredible how much
> > crap is
> > > > > > out there. so it is crucial not to leave *anything* to
> > interpretation in
> > > > > > a spec.
> > > > >
> > > > >
> > > > >
> > > > > couldn't KConfigGroup just escape the escape chars, and if you
> want to
> > > > change KEntryMap, I don't really see a problem, but can't that wait
> > until
> > > > after 4.0 ? It's not like we have any other backends right now.
> > > >
> > > > I finally got a clean checkout to compile, and it looks like the
> fixes
> > that
> > > > were made fixed the symptoms instead of the problems, try this patch
> and
> > see
> > > > if it fixes all the problems, if it does I can commit it, I need to
> know
> > > > within the next couple of hours, after that I'm off to work.
> > > > Oh and what were those tests added to kconfigtest.cpp supposed to
> test?
> > > > Because they failed here on a clean checkout. I don't think they are
> > right,
> > > > but I'm not sure what they are supposed to be testing, so I don't
> know
> > if I
> > > > should fix the tests or fix KConfigGroup to pass the tests.
> > > >
> > > The tests were added to test what's broken now, and in this thread we
> > > are pondering how to fix it. It's not that easy so please don't just
> > > commit something.
> > >
> > I know what was being pondered on this thread, but if I don't understand
> > what you think is broken then I can't fix it can I?
> >
>
> 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.

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.

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20071008/b0cd6b1a/attachment.htm>