extragear/multimedia/amarok/src

[Jeff Mitchell]

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
>