Review Request: Provide static KFileDialog::getSave* functions with "confirm overwrite" option

[David Jarvie]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/822/#review1473
-----------------------------------------------------------


I've created a new patch, but despite many attempts, can't upload it. It addresses the startDir issue. I've still left the current default for confirm-overwrite unchanged, to keep the behaviour identical for Windows. If this behaviour is considered a bug, I can alter this. However, I'd say that the current non-Windows implementation is a bug in that it does _NOT_ ask for confirmation.

The changed part of the patch is:

@@ -710,9 +737,19 @@
 KUrl KFileDialog::getSaveUrl(const KUrl& dir, const QString& filter,
                              QWidget *parent, const QString& caption)
 {
+    //TODO KDE5: replace this method by the method below (with default parameter values in declaration)
+    // Set overwrite mode for backwards compatibility
+    Options options = (KFileDialogPrivate::isNative() && (!dir.isValid() || dir.isLocalFile()))
+                    ? ConfirmOverwrite : Options(0);
+    return getSaveUrl(dir, filter, parent, caption, options);
+}
+
+KUrl KFileDialog::getSaveUrl(const KUrl& dir, const QString& filter,
+                             QWidget *parent, const QString& caption, Options options)
+{
     if (KFileDialogPrivate::isNative() && (!dir.isValid() || dir.isLocalFile())) {
         const QString fileName( KFileDialog::getSaveFileName(
-            dir, filter, parent, caption) );
+            dir, filter, parent, caption, options) );
         return fileName.isEmpty() ? KUrl() : KUrl::fromPath(fileName);
     }
     bool defaultDir = dir.isEmpty();


- David


On 2009-06-10 14:44:59, David Jarvie wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/822/
> -----------------------------------------------------------
> 
> (Updated 2009-06-10 14:44:59)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Add new overloads of the static KFileDialog::getSave* functions to allow "confirm overwrite" and other options to be specified. Ideally the new variants would replace the existing functions, but that is not possible for binary compatibility reasons during KDE4.
> 
> The new functions will allow the static KFileDialog functions to be used in cases where "confirm overwrite" is needed, without the developer having to construct a KFileDialog and set it up manually (thereby losing the convenience of the static functions which among other things automatically call native file dialogs when appropriate). This will improve consistency between applications as well as reducing developer work.
> 
> KEncodingFileDialog would benefit from a similar change, but it would need access to KFileDialogPrivate::isNative() to do this properly - this could presumably be provided via a new protected method in KFileDialog (being non-virtual, presumably it wouldn't be BIC?).
> 
> This change is proposed for KDE 4.4.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kio/kfile/kfiledialog.h 951469 
>   /trunk/KDE/kdelibs/kio/kfile/kfiledialog.cpp 951469 
> 
> Diff: http://reviewboard.kde.org/r/822/diff
> 
> 
> Testing
> -------
> 
> getSaveFileName() and getSaveUrl() tested successfully in KAlarm.
> 
> 
> Thanks,
> 
> David
> 
>