[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: PATCH: KonqRun::askSave and URLs containing '%'
From:       David Faure <david () mandrakesoft ! com>
Date:       2001-08-29 19:20:47
[Download RAW message or body]

On Mercredi 29 Août 2001 23:57, Maarten ter Huurne wrote:
> On Saturday 25 August 2001 16:04, Carsten Pfeiffer wrote:
> > On Sat, Aug 25, 2001 at 04:30:33PM +0000, Maarten ter Huurne wrote:
> > > Where should this functionality be located? As a function, as a static
> > > method or as an instance method? And in case of a method, in which class?
> > > Should it be part of KDE or should we try to get TrollTech to put it in
> > > Qt? Since it is a more general replacement for QString::arg, my opinion
> > > is that it belongs in QString.
> >
> > Until QString offers this, we could put it into KStringHandler (kdecore).
> 
> Attached patches add an "arg" method to KStringHandler, in the way David 
> suggested (using default arguments). They are diffs from the KDE 2.2 code.
> 
> I think this is a convenient, safe and efficient way to substitute multiple 
> arguments. I verified the code by reviewing and running a few tests, but 
> please double check it just in case.

Looks good to me, except for the "if the arg is null then %n remains" specification.
Shouldn't null give an empty substitution ? Or "(null)" maybe. But a %n in a
message to be read by the user is going to look very ugly.

Note: your switch() lacks a "default:" case.
Note2: you don't have to use pointers with QString. QString a = b is very cheap
(it's a shared class).

Thanks for your contribution !

-- 
David FAURE, david@mandrakesoft.com, faure@kde.org
http://perso.mandrakesoft.com/~david/ , http://www.konqueror.org/
KDE, Making The Future of Computing Available Today

[prev in list] [next in list] [prev in thread] [next in thread]