KDE/kdevplatform

[Andreas Pakulat]

On 11.05.09 10:45:05, David Nolden wrote:
> Am Sonntag 10 Mai 2009 23:47:14 schrieb Amilcar do Carmo Lucas:
> > SVN commit 966293 by aclu:
> >
> > Krazy
> >
> >  M  +1 -1      plugins/subversion/svninternaljobbase.cpp
> >  M  +1 -1      util/executecompositejob.h
> >  M  +1 -1      vcs/dvcs/dvcsplugin.cpp
> >
> >
> > --- trunk/KDE/kdevplatform/plugins/subversion/svninternaljobbase.cpp
> > #966292:966293 @@ -170,7 +170,7 @@
> >                  sendFirstDelta = false;
> >                  notifyString=i18n("Transmitting file data ");
> >              } else {
> > -                notifyString=".";
> > +                notifyString='.';
> >              }
> >              break;
> >          case svn_wc_notify_blame_revision:
> > --- trunk/KDE/kdevplatform/util/executecompositejob.h #966292:966293
> > @@ -33,7 +33,7 @@
> >  public:
> >      ExecuteCompositeJob(QObject* parent, const QList<KJob*>& jobs);
> >      virtual void start();
> > -public slots:
> > +public Q_SLOTS:
> >      virtual void slotResult(KJob* job);
> >  protected:
> >      virtual bool doKill();
> > --- trunk/KDE/kdevplatform/vcs/dvcs/dvcsplugin.cpp #966292:966293
> > @@ -425,7 +425,7 @@
> >              file = url.toLocalFile();
> >
> >          if (file.isEmpty())
> > -            file = ".";
> > +            file = '.';
> >
> >          args << file;
> >          kDebug() << "url is: " << url << "job->getDirectory(): " <<
> > workingDir << " file is: " << file;
> 
> I've got to say, I don't like this specific kind of krazy fixes.
> 
> - Q_SLOTS is plain ugly

Yet, it disambiguates the far too general "slots" macro. This is not
negotiable.

> - assigning a character to a string may be allowed in the case of QString, but 
> semantically it's wrong and ugly, and additionally to that we've already seen 
> such changes cause new bugs. It's a micro optimization that is irrelevant in 
> these use-cases, and I think it shouldn't be done on a general basis, given 
> the disadvantages (Chance of new bugs, semantic wrongness)

Well, I'm not seeing us divert from QString in the mid or long term, so
thats fine. However I agree that in most cases this is micro-optimization
thats totally useless. So I suggest to simply mark these places as
krazy-excluded (via an inline-comment) unless profiling shows that it
really is a bottle-neck.

And regarding the "semantically wrong and ugly", I don't follow you at all.
Adding a character to a list of characters is just fine with me. It works
in the real world (hey thats how I write), it works when typing files so I
don't see any reason (except arbitrary limits imposed by the language or
other string classes) why this shouldn't work in code.

Andreas

-- 
You will soon forget this.