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

List:       kde-core-devel
Subject:    Re: Fwd: KFileDialog combined keyword/filename suggestion
From:       David Faure <faure () kde ! org>
Date:       2009-03-13 0:29:15
Message-ID: 200903130129.15685.faure () kde ! org
[Download RAW message or body]

On Wednesday 11 March 2009, Jonathan Marten wrote:
> Albert Astals Cid <aacid@kde.org> writes:
> > You forgot to attach the patch :D
> 
> It's attached to the bug report
> (https://bugs.kde.org/show_bug.cgi?id=186230).  Follows here too...

No objections from me. (Can't be 100% sure that there are no regressions, of course,
I don't know that code enough).

Just one thing that could be simplified:
+            startDir.adjustPath(KUrl::RemoveTrailingSlash);
+            filename = startDir.fileName();
+            startDir.setFileName(QString());
would be more usually done as:
             filename = startDir.fileName();  // KUrl::IgnoreTrailingSlash is default anyway
             startDir.setPath(startDir.directory());

Same for:
+                KUrl u(url);
+                u.setFileName("");
I would do u.setPath(u.directory()) instead. Especially since that's exactly what
the if() tests, so it would be more consistent.

-- 
David Faure, faure@kde.org, sponsored by Qt Software @ Nokia to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
[prev in list] [next in list] [prev in thread] [next in thread]