extragear/multimedia/amarok/src

Andrew Turner andrewturner512 at googlemail.com
Wed Sep 20 12:35:21 UTC 2006


Jeff,

Well I know of certain bits of code where it definitely should no be
isEmpty(). Whether those are all correct, I don't know, but changing
them over without a lot of thinking can lead to many tiny regressions.
I advise caution.

Andrew

On 20/09/06, Jeff Mitchell <kde-dev at emailgoeshere.com> wrote:
> Andrew--
>
> Right...I wasn't concerned that the commits changed the behavior of
> the existing code, but rather, should we change the behavior anyways.
> I've run into situations before where a string is empty but not null,
> but should be treated the same way in either case.  It was more a
> "should this be our policy" type thing :-)
>
> --Jeff
>
> Quoting Andrew Turner <andrewturner512 at googlemail.com>:
>
> > Jeff,
> >
> > If you look again at the commit message,
> >
> > foo == QString::null
> > has been turned into
> > foo.isNull()
> >
> > These two are identical in result, so the new code is just as valid as
> > what was there before. Obviously testing for null strings is not
> > always appropriate, but that doesn't make this commit any more/less
> > valid.
> >
> > Andrew
> >
> > On 20/09/06, Alexandre Oliveira <aleprjlists at gmail.com> wrote:
> >> isNull() is true if the string has never been assigned anything. I was
> >> not willing to check whether using isEmpty() would work, as some
> >> bizarre code might actually rely on the difference.
> >> I agree it should be changed, though.
> >>
> >> On 9/19/06, Jeff Mitchell <kde-dev at emailgoeshere.com> wrote:
> >> > I just caught this, when reviewing all the SVN commits from when
> >> I was gone:
> >> >
> >> > AFAIK, we should actually not be comparing to QString::isNull(); we
> >> > should be comparing to QString::isEmpty().  The former case does not
> >> > handle a non-null but empty string ( "" ), while the latter case
> >> > handles both.  Unless there's a reason we might want to allow an empty
> >> > string in the comparison.
> >> >
> >> > --Jeff
> >> >
> >> > Quoting Alexandre Pereira de Oliveira <aoliveira at kdemail.net>:
> >> >
> >> > > SVN commit 583697 by aoliveira:
> >> > >
> >> > > Still on englishbreakfastnetwork suggested fixes
> >> > >  Still some " " to ' ' changes;
> >> > >  Use QString::isNull() instead of comparing to QString::null;
> >> > >  Some typos on comments;
> >> > >  Some const correctness;
> >> > >
> >> > >
> >> > >  M  +1 -1      amarokcore/crashhandler.cpp
> >> > >  M  +2 -2      app.cpp
> >> > >  M  +1 -1      atomicurl.cpp
> >> > >  M  +5 -5      collectiondb.cpp
> >> > >  M  +1 -1      collectiondb.h
> >> > >  M  +1 -1      database_refactor/collectiondb.cpp
> >> > >  M  +5 -5      devicemanager.cpp
> >> > >  M  +5 -5      devicemanager.h
> >> > >  M  +1 -1      engine/gst10/gstengine.cpp
> >> > >  M  +128 -128  engine/helix/helix-sp/helix-sp.cpp
> >> > >  M  +14 -14    engine/helix/helix-sp/hspcontext.cpp
> >> > >  M  +7 -7      engine/helix/helix-sp/hsphook.cpp
> >> > >  M  +7 -7      engine/nmm/HostListItem.cpp
> >> > >  M  +2 -2      ktrm.cpp
> >> > >  M  +3 -3      lastfm.cpp
> >> > >  M  +3 -3      lastfm.h
> >> > >  M  +4 -4      mediabrowser.cpp
> >> > >  M  +1 -1      mediadevice/daap/daapclient.cpp
> >> > >  M  +2 -2      mediadevice/generic/genericmediadevice.cpp
> >> > >  M  +3 -3      mediadevice/ipod/ipodmediadevice.cpp
> >> > >  M  +4 -4      mediumpluginmanager.cpp
> >> > >  M  +1 -1      metabundle.cpp
> >> > >  M  +3 -3      metadata/m4a/itunesalbbox.cpp
> >> > >  M  +3 -3      metadata/m4a/itunesartbox.cpp
> >> > >  M  +3 -3      metadata/m4a/itunescmtbox.cpp
> >> > >  M  +3 -3      metadata/m4a/itunescvrbox.cpp
> >> > >  M  +3 -3      metadata/m4a/itunesdaybox.cpp
> >> > >  M  +3 -3      metadata/m4a/itunesdiskbox.cpp
> >> > >  M  +3 -3      metadata/m4a/itunesgenbox.cpp
> >> > >  M  +3 -3      metadata/m4a/itunesgrpbox.cpp
> >> > >  M  +3 -3      metadata/m4a/itunesnambox.cpp
> >> > >  M  +3 -3      metadata/m4a/itunestmpobox.cpp
> >> > >  M  +3 -3      metadata/m4a/itunestrknbox.cpp
> >> > >  M  +3 -3      metadata/m4a/ituneswrtbox.cpp
> >> > >  M  +1 -1      osd.cpp
> >> > >  M  +1 -1      playlist.cpp
> >> > >  M  +1 -1      playlist.h
> >> > >  M  +3 -3      playlistbrowser.cpp
> >> > >  M  +3 -3      playlistbrowseritem.cpp
> >> > >  M  +4 -4      scancontroller.cpp
> >> > >  M  +1 -1      smartplaylisteditor.cpp
> >> > >  M  +1 -1      statusbar/statusbar.cpp
> >> > >  M  +2 -2      vis/xmmswrapper/xmmswrapper.cpp
> >> > >
> >> > >
> >> > >
> >> >
> >> >
> >> > _______________________________________________
> >> > Amarok mailing list
> >> > Amarok at kde.org
> >> > https://mail.kde.org/mailman/listinfo/amarok
> >> >
> >> _______________________________________________
> >> Amarok mailing list
> >> Amarok at kde.org
> >> https://mail.kde.org/mailman/listinfo/amarok
> >>
> > _______________________________________________
> > Amarok mailing list
> > Amarok at kde.org
> > https://mail.kde.org/mailman/listinfo/amarok
> >
>
>
> _______________________________________________
> Amarok mailing list
> Amarok at kde.org
> https://mail.kde.org/mailman/listinfo/amarok
>



More information about the Amarok mailing list