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

List:       kde-core-devel
Subject:    Re: [PATCH] ReadWritePart::queryClose()
From:       John Firebaugh <jfirebaugh () kde ! org>
Date:       2002-12-11 6:09:49
[Download RAW message or body]

On Sunday 01 December 2002 4:51, you wrote:
> (You removed the kde-core-devel cc - intentionnally?)

Oops, no, was using a bad web-mail interface.

> On Monday 02 December 2002 00:17, you wrote:
> > But saveAs will only be called from queryClose if the document is new, in
> > which case there won't be a temp file, right?
>
> Not necessarily. E.g. in KOffice when you import a file (from another
> format), you get a document with no URL, so that when you press Save, it
> will go to "Save As", to force you to save it as a native format document.
> Same thing if you get the "wanna save?" dialog -> you then have to specify
> a filename to save that stuff to.

But is there a temp file in that case?

> Ok, this doesn't mean that your change necessarily breaks KOffice, since
> it reimplements quite a lot of that stuff (for various other reasons), but
> anyway, I think it's wrong to assume that only "created from empty"
> documents have no URL.

Well, that's basically exactly what the code does:

if (m_url.isEmpty())
{
    KURL url = KFileDialog::getSaveURL();
    ...
    return saveAs( url );
}
return save();

> > >I would suggest moving m_bClosing=true before the saveAs() call, as it
> > > was before - after all queryClose() is about closing in all cases,
> > > right?
> >
> > When queryClose is called on it's own (not from closeURL), it isn't
> > guaranteed that the document is really closed afterward (something else
> > could cancel the close).
> >
> > Therefore the temp file must remain, which is why I only set m_bClosing
> > to true from closeURL.
>
> Hmm. Ok. In fact the temp file will get deleted anyway - by
> ReadOnlyPart::closeURL, which is called upon destruction.

Yes. BTW, how does that work when the file is being uploaded asynchronously? 
I.e. what happens if the part is deleted before the upload finishes, and thus 
the temp file is removed? Does kio make a temp copy of its own before this 
can happen? Or is the app supposed to wait for the upload to finish?

> Hmm, difficult indeed, at the Part level we have no concept of the
> "window-level closing operation". So it can be cancelled after
> queryClose(), even if queryClose() said ok (yes or no).... Maybe a solution
> is to introduce another method, say cancelClose(), which lets the app tell
> the part that "we're not closing anymore". So the part could set m_bClosing
> in queryClose(), but would reset it in cancelClose(). And closeURL()
> wouldn't do anything if m_bClosing - since that means queryClose was
> already called, so the user was already prompted.

Seems like a kludgy solution. I'd prefer an overloaded closeURL() with a flag 
that controls whether or not to prompt.

Actually, apps that use the queryClose() method could just make sure to call 
ReadOnlyPart::closeURL() instead of ReadWritePart::closeURL(), or leave it to 
the destructor to call, which is what I have kate doing.

How about this patch?

-John
["queryClose.diff" (text/x-diff)]

Index: part.cpp
===================================================================
RCS file: /home/kde/kdelibs/kparts/part.cpp,v
retrieving revision 1.119
diff -u -3 -p -r1.119 part.cpp
--- part.cpp	17 Jul 2002 18:01:54 -0000	1.119
+++ part.cpp	11 Dec 2002 05:59:13 -0000
@@ -462,39 +462,53 @@ void ReadWritePart::setModified()
   setModified( true );
 }
 
+bool ReadWritePart::queryClose()
+{
+  if ( !isReadWrite() || !isModified() )
+    return true;
+
+  int res = KMessageBox::warningYesNoCancel( widget(),
+          i18n( "The document \"%1\" has been modified.\n"
+                "Do you want to save it?" ).arg( url().fileName() ),
+          i18n( "Save Document?" ), KStdGuiItem::save(), KStdGuiItem::discard() );
+
+  switch(res) {
+  case KMessageBox::Yes :
+    if (m_url.isEmpty())
+    {
+        KURL url = KFileDialog::getSaveURL();
+        if (url.isEmpty())
+        {
+          m_bClosing = false;
+          return false;
+        }
+        return saveAs( url );
+    }
+    return save();
+  case KMessageBox::No :
+    m_bClosing = false;
+    return true;
+  default : // case KMessageBox::Cancel :
+    m_bClosing = false;
+    return false;
+  }
+}
+
 bool ReadWritePart::closeURL()
 {
   abortLoad(); //just in case
-  if ( m_bModified && m_bReadWrite )
+  if ( isReadWrite() && isModified() )
   {
-    int res = KMessageBox::warningYesNoCancel( widget(),
-            i18n( "The document \"%1\" has been modified.\n"
-                  "Do you want to save it?" ).arg( url().fileName() ),
-            i18n( "Save Document?" ), KStdGuiItem::save(), KStdGuiItem::discard() );
-
-    switch(res) {
-    case KMessageBox::Yes :
-      m_bClosing = true; // remember to clean up the temp file
-      if (m_url.isEmpty())
-      {
-          KURL url = KFileDialog::getSaveURL();
-          if (url.isEmpty())
-          {
-            m_bClosing = false;
-            return false;
-          }
-          return saveAs( url );
-      }
-      return save();
-    case KMessageBox::No :
-      setModified( false ); // the user isn't interested in the changes, forget them
-      return true;
-    default : // case KMessageBox::Cancel :
-      return false;
-    }
+    m_bClosing = true; // remember to clean up the temp file
+    return queryClose();
   }
   // Not modified => ok and delete temp file.
   return ReadOnlyPart::closeURL();
+}
+
+bool ReadWritePart::closeURL( bool promptToSave )
+{
+  return promptToSave ? closeURL() : ReadOnlyPart::closeURL();
 }
 
 bool ReadWritePart::save()
Index: part.h
===================================================================
RCS file: /home/kde/kdelibs/kparts/part.h,v
retrieving revision 1.93
diff -u -3 -p -r1.93 part.h
--- part.h	15 Jul 2002 09:25:31 -0000	1.93
+++ part.h	11 Dec 2002 05:59:14 -0000
@@ -531,15 +531,43 @@ public:
   bool isModified() const { return m_bModified; }
 
   /**
+   * If the document has been modified, ask the user to save changes.
+   * This method is meant to be called from @ref KMainWindow::queryClose().
+   * It will also be called from @ref closeURL().
+   *
+   * @return true if closeURL() can be called without the user losing
+   * important data, false if the user chooses to cancel.
+   *
+   * @since 3.2
+   */
+  // TODO: Make virtual for KDE 4
+  bool queryClose();
+
+  /**
    * Called when closing the current url (e.g. document), for instance
    * when switching to another url (note that @ref openURL() calls it
    * automatically in this case).
+   *
    * If the current URL is not fully loaded yet, aborts loading.
-   * Reimplemented from ReadOnlyPart, to handle modified parts
-   * (and suggest saving in this case, with yes/no/cancel).
+   *
+   * If @ref isModified(), @ref queryClose() will be called.
+   *
    * @return false on cancel
    */
   virtual bool closeURL();
+
+  /**
+   * Call this method instead of the above if you need control if
+   * the save prompt is shown. For example, if you call @ref queryClose()
+   * from @ref KMainWindow::queryClose(), you would not want to prompt
+   * again when closing the url.
+   *
+   * Equivalent to promptToSave ? @ref closeURL() : @ref ReadOnlyPart::closeURL()
+   *
+   * @since 3.2
+   */
+  // TODO: Make virtual for KDE 4
+  bool closeURL( bool promptToSave );
 
   /**
    * Save the file to a new location.


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