Bug 49270 - session management doesn't work
Summary: session management doesn't work
Status: RESOLVED FIXED
Alias: None
Product: kate
Classification: Applications
Component: kwrite (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: HI major
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
: 64524 (view as bug list)
Depends on:
Blocks:
 
Reported: 2002-10-17 08:19 UTC by John Firebaugh
Modified: 2005-01-22 15:29 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
KParts::queryClose implementation (2.29 KB, patch)
2003-01-27 06:30 UTC, Hamish Rodda
Details
The kwrite part of this patch. (554 bytes, patch)
2003-01-27 06:30 UTC, Hamish Rodda
Details
Next revision of my proposed patch (3.15 KB, patch)
2003-03-08 07:10 UTC, Hamish Rodda
Details

Note You need to log in before you can comment on or make changes to this bug.
Description John Firebaugh 2002-10-17 08:19:01 UTC
Version:           4.1 (using KDE 3.0.7 (KDE 3.1 beta1))
Installed from:    compiled sources
Compiler:          gcc version 2.95.4 20011002 (Debian prerelease)
OS:          Linux (ppc) release 2.4.19-powerpc

All instances of kwrite are restored with an empty document instead of the document open when exiting the session. If you watch carefully, you can see the documents closing before the session exits -- this is probably the problem.
Comment 1 Lubos Lunak 2002-10-18 12:58:49 UTC
 The problem is not kwrite specific, Kate exhibits problems with session restore too. E.g. start  
Kate with no files open (i.e. only Untitled 1), type something, start logout, confirm file save and  
save it. On next KDE startup empty Untitled 1 is open. If the file is saved explicitly using Ctrl+S 
before logging out, it's restored correctly (it's also interesting to note that in the Ctrl+S case a 
dialog warning about overwriting an existing file opens while in the first case it does not). Also, 
if there are any Ctrl+S saved files open when editing one Untitled, they're not in the documents 
list when confirming save, and are also not restored. 
 
 I'm assuming that either Kate closes the files when doing session save (which is wrong, 
session shutdown may be cancelled), or it tries handling session file saving completely on its 
own and isn't very good at it (which is not very good either, even if it worked). 
 
Comment 2 John Firebaugh 2002-12-01 04:27:26 UTC
This bug was introduced in revision 1.32 of kwritemain.cpp. The cause is calling 
KateDocument::closeURL() from queryClose(). KateDocument::closeURL() calls 
ReadWritePart::closeURL(), which first displays the "Save Changes?" dialog, and then closes the 
URL if appropriate. We want the first part, but not the second. 
 
The ideal solution is to factor out the "Save Changes?" part of ReadWritePart::closeURL() into a 
new function (suggested name: queryClose()). It probably should be virtual, but can survive 
until 4.0 without that, I think. It can then be called from both ReadWritePart::closeURL() and 
KWrite::queryClose(). 
 
Comments? 
Comment 3 Hamish Rodda 2003-01-27 06:30:24 UTC
Created attachment 815 [details]
KParts::queryClose implementation

I agree with John's assessment, so I implemented it :)	Works fine here.  
(Although I am seeing a few other problems with kate, they're 99% likely to be
other bugs).

Ok to commit?  backport?
Comment 4 Hamish Rodda 2003-01-27 06:30:54 UTC
Created attachment 816 [details]
The kwrite part of this patch.
Comment 5 David Faure 2003-01-27 11:06:27 UTC
Subject: Re:  session management doesn't work

On Monday 27 January 2003 06:30, you wrote:
> I agree with John's assessment, so I implemented it :)	Works fine here.  
> (Although I am seeing a few other problems with kate, they're 99% likely to be
> other bugs).
> 
> Ok to commit?  backport?

Now I'm surprised. I thought I saw a patch on kde-core-devel which implemented
queryClose in KParts::ReadWritePart already.

Ah yes. The patch was from John :)
http://lists.kde.org/?l=kde-core-devel&m=103958666221160&w=2
I thought it was applied by now.

Comparing your patches: John's had an additionnal closeURL(bool promptToSave)
which I didn't like very much (but accepted nonetheless). Its docu said "to avoid
prompting twice, when prompting would be done by both queryClose and closeURL".
How is this solved in Hamish's patch?

Anyway, I'm fine with either version - please sort out which one you want to
commit :)

Comment 6 Hamish Rodda 2003-01-27 11:21:04 UTC
Subject: Re:  session management doesn't work

On Monday 27 January 2003 21:06, David Faure wrote:
> On Monday 27 January 2003 06:30, you wrote:
> > I agree with John's assessment, so I implemented it :)	Works fine here.
> > (Although I am seeing a few other problems with kate, they're 99% likely
> > to be other bugs).
> >
> > Ok to commit?  backport?
>
> Now I'm surprised. I thought I saw a patch on kde-core-devel which
> implemented queryClose in KParts::ReadWritePart already.
>
> Ah yes. The patch was from John :)
> http://lists.kde.org/?l=kde-core-devel&m=103958666221160&w=2
> I thought it was applied by now.

Doesn't appear so to me...

> Comparing your patches: John's had an additionnal closeURL(bool
> promptToSave) which I didn't like very much (but accepted nonetheless). Its
> docu said "to avoid prompting twice, when prompting would be done by both
> queryClose and closeURL". How is this solved in Hamish's patch?

Ah, I guess it's not, except that I thought this was only for when you want to 
not close the document?  I didn't look too hard at the issues with this.

> Anyway, I'm fine with either version - please sort out which one you want
> to commit :)

John, any reason you didn't commit yours?

Cheers,
Hamish.

Comment 7 John Firebaugh 2003-01-28 07:52:35 UTC
> John, any reason you didn't commit yours? 
 
Only that I, like David, wasn't completely satisfied with it. But, of all the suggested solutions, it 
still seems like the best to me. 
 
I'll commit in the next few days. 
Comment 8 Hamish Rodda 2003-01-28 08:36:26 UTC
Now understanding the issue (I think :), a suggestion: you could add the bool argument 
to the queryClose() function instead (eg. queryClose(bool promptOnClose = false)) 
and store the result for use in closeURL() (or inside queryClose itself) instead of 
duplicating closeURL.  It looks like you'd need to use Qt's property system to store it 
though, it doesn't seem to me that there's a d pointer available... or make readonlypart 
expose a new bool from it's private class though a function. 
Comment 9 John Firebaugh 2003-01-30 06:37:32 UTC
I'm leaving on a trip very soon so actually I'm not going to have a chance to double-check and 
commit this patch. Hamish and David, I guess you two should go ahead and apply whatever 
variation you agree on. 
Comment 10 Hamish Rodda 2003-03-08 07:10:53 UTC
Created attachment 1135 [details]
Next revision of my proposed patch

This patch incorporates a different approach to preventing the queryClose
prompt from being displayed twice.  queryClose now has a parameter,
promptOnClose, which can be set to false to prevent closeURL from calling
queryClose.  This avoids the need for a second closeURL() function.  This is
implemented with a static QMap as there is no d-pointer.

Comments, ok to apply? (backport?)
Comment 11 Christoph Cullmann 2003-03-22 23:32:55 UTC
perhaps you should send the patch to core-devel, or we never get a reply ;) 
Comment 12 John Firebaugh 2003-03-31 09:00:44 UTC
Subject: kdebase/kate/app

CVS commit by firebaugh: 

Fix bug #49270, session management doesn't work: use the new ReadWritePart method queryClose() instead of closeURL().

CCMAIL: 49270-done@bugs.kde.org


  M +3 -3      kwritemain.cpp   1.63


--- kdebase/kate/app/kwritemain.cpp  #1.62:1.63
@@ -131,6 +131,6 @@ void KWrite::loadURL(const KURL &url)
 bool KWrite::queryClose()
 {
-  if (!(kateView->document()->views().count() == 1)) return true;
-  return kateView->document()->closeURL();
+  return kateView->document()->views().count() != 1
+      || kateView->document()->queryClose();
 }
 


Comment 13 John Firebaugh 2003-09-22 03:10:26 UTC
*** Bug 64524 has been marked as a duplicate of this bug. ***
Comment 14 Jens Dagerbo 2003-11-04 04:54:41 UTC
This problem is back in HEAD.
Comment 15 Christoph Cullmann 2003-11-04 21:41:38 UTC
for kate or kwrite ? kate works here perfect
Comment 16 Christoph Cullmann 2003-11-04 23:30:03 UTC
hmm, true, kwrite session restore was borked (not showing the reopened files + the other bug with the crash after closing the kwrite)
fixed now in cvs, not the perfect fix, but works
Comment 17 Damir Perisa 2005-01-22 15:29:51 UTC
this fix seems to only work for kde:

i run xfce4-session and kwrite does NOT properly save it's state (the file that was open does not restore but an empty kwrite loads)

see xfce4 bug here: http://bugzilla.xfce.org/show_bug.cgi?id=733

please reopen this bug