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

List:       kde-devel
Subject:    Re: Dolphin Patch ?
From:       David Faure <faure () kde ! org>
Date:       2008-06-30 8:47:40
Message-ID: 200806301047.41142.faure () kde ! org
[Download RAW message or body]

On Saturday 28 June 2008, Peter Penz wrote:
> Hi Andreas,
> 
> thanks for the patch!
> 
> On Saturday, 28. June 2008 01:36:47 Andreas Scherf wrote:
> > Hello,
> > i changed one thing in the selection part of the dolphin main window.
> > Everytime 2 files/dirs are selected dolphin searches for "kompare".
> > I think that we could test this only once to safe time. If somebody
> > install/removes compare while running dolphin he couldn't use kompare.
> > But i think the runtime costs are reduced. Ok ?
> 
> I'm not sure: the runtime costs for comparing are decreased, but the runtime 
> costs for starting up Dolphin has been increased now.
> 
> I'd suggest doing it like this:
> instead of the original code (which does an unnecessary comparison anyway):
> 
> if (selectedUrlsCount == 2) {
>   const bool kompareInstalled 
> = !KGlobal::dirs()->findExe("kompare").isEmpty();
>   compareFilesAction->setEnabled(selectedUrlsCount == 2 && kompareInstalled);
> }
> 
> I'd suggest something like this:
> 
> if (selectedUrlsCount == 2) {
> 	compareFilesAction->setEnabled(isKompareInstalled());
> }
> 
> bool DolphinMainWindow::isKompareInstalled() const
> {
> 	static bool initialized = false;
> 	static bool installed = false;
> 	if (!initialized) {
> 		installed= !KGlobal::dirs()->findExe("kompare").isEmpty();
> 		initialized = true;
> 	}
> 	return installed;
> }
> 
> What do you think?

Looks good to me.

-- 
David Faure, faure@kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
 
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
[prev in list] [next in list] [prev in thread] [next in thread]