Bug 152761 - No session management in KDE 4 - Sessions / directory / tabs not restored on login
Summary: No session management in KDE 4 - Sessions / directory / tabs not restored on ...
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: unspecified Linux
: HI normal with 123 votes (vote)
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
: 153489 154047 156995 157688 159034 162589 167129 167628 169982 170221 173455 179378 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-11-23 09:39 UTC by Will Stephenson
Modified: 2009-05-12 10:37 UTC (History)
27 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Implement basic session management for KDE4 konsole (13.54 KB, patch)
2008-09-09 19:32 UTC, Stefan Becker
Details
Proposed patch for upstream merge (15.51 KB, patch)
2008-09-10 23:58 UTC, Stefan Becker
Details
Proposed patch for upstream merge (15.41 KB, patch)
2008-09-11 00:23 UTC, Stefan Becker
Details
Proposed patch for upstream merge (30.58 KB, patch)
2008-09-12 21:39 UTC, Stefan Becker
Details
Proposed patch for upstream merge (33.84 KB, patch)
2008-09-22 20:01 UTC, Stefan Becker
Details
KDE SVN trunk revisions 867154 to 867477 changes backported to 4.1.1 (35.69 KB, patch)
2008-10-06 05:51 UTC, Stefan Becker
Details
Fix restore from invalid session data (1.86 KB, patch)
2008-10-06 07:16 UTC, Stefan Becker
Details
Fix restore from invalid session data (2.26 KB, patch)
2008-10-06 17:18 UTC, Stefan Becker
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Will Stephenson 2007-11-23 09:39:48 UTC
Version:            (using KDE Devel)
OS:                Linux

Tabs don't appear to be session managed any more.  A session file for a KDE3 konsole shows a lot more state than a KDE4 konsole.
Comment 1 Robert Knight 2007-12-11 05:14:44 UTC
Confirmed.  At present I do not plan to implement more sophisticated session management until KDE 4.1

Comment 2 Robert Knight 2007-12-11 05:16:44 UTC
*** Bug 153489 has been marked as a duplicate of this bug. ***
Comment 3 Robert Knight 2007-12-14 21:25:37 UTC
*** Bug 154047 has been marked as a duplicate of this bug. ***
Comment 4 Lubos Lunak 2008-01-22 19:40:54 UTC
While you'll be at fixing session management, KUniqueApplication::newInstance() override will also need some work (see its docs).
Comment 5 Robert Knight 2008-01-22 19:57:35 UTC
> KUniqueApplication::newInstance() override will also need some work (see its docs)

What is wrong and/or missing?  It isn't obvious to me from looking at the API documentation.
Comment 6 Lubos Lunak 2008-01-24 14:52:38 UTC
I committed changes for docs for KUniqueApplication::newInstance() in trunk. Konsole's newInstance() doesn't handle starting from restored session, and also the case when you reuse a window (new tab) needs to act like KUniqueApplication::newInstance().

Comment 7 Robert Knight 2008-01-31 00:18:42 UTC
*** Bug 156995 has been marked as a duplicate of this bug. ***
Comment 8 Robert Knight 2008-02-12 00:43:26 UTC
*** Bug 157688 has been marked as a duplicate of this bug. ***
Comment 9 Robert Knight 2008-03-10 03:20:51 UTC
*** Bug 159034 has been marked as a duplicate of this bug. ***
Comment 10 Pino Toscano 2008-05-25 11:16:39 UTC
*** Bug 162589 has been marked as a duplicate of this bug. ***
Comment 11 jos poortvliet 2008-05-30 07:41:37 UTC
Any updates on this? Won't make it into 4.1 I suppose?
Comment 12 Robert Knight 2008-05-31 02:26:47 UTC
> Any updates on this? Won't make it into 4.1 I suppose? 

No changes have landed yet.  Still scheduled for 4.1.  
Comment 13 jos poortvliet 2008-05-31 09:42:33 UTC
Still scheduled? So good chance it'll get done? That's great news!

Konsole really became a lot better since you started working on it, there are just a few minor annoyances left for it to become perfect.

Maybe you should delay fixing them, as you won't have much to do for 4.2 - or do you have some amazing new things in mind? hehe :D
Comment 14 Robert Knight 2008-07-21 14:37:52 UTC
*** Bug 167129 has been marked as a duplicate of this bug. ***
Comment 15 Pino Toscano 2008-07-29 00:10:35 UTC
*** Bug 167628 has been marked as a duplicate of this bug. ***
Comment 16 Ciprian 2008-07-31 16:14:13 UTC
4.1... No session management?! :(
Comment 17 Stefan Becker 2008-07-31 22:59:28 UTC
Yup, 4.1 version still doesn't support session management. Bummer :-(

Even the workaround of using "konsole" from a script directly, as "konsole --new-tab" only seems to open tabs in the initial instance. This is quite annoying and a show stopper for moving to KDE4 on my main workstation, as my session has 4 konsole windows with a total of 20 tabs/working directories in them.
Comment 18 Richard Hartmann 2008-08-07 15:06:57 UTC
Just wanted to report the same issue for Konsole 2.1.

Same as Stefan Becker, this single issue might force me to go back to KDE3, which I don't want to do.. 

Is there any ETA on this? How much work are we talking about?
Comment 19 devsk 2008-08-11 12:21:48 UTC
This is a showstopper for me as well! A kde session restart/crash, for example, will need me to open at least 10 new windows and multiple tabs in them in my development environment.
Comment 20 devsk 2008-08-20 17:34:25 UTC
Any idea if this is going to be in 4.1.1? If someone can even suggest a workaround (apart from open 10 windows and 20 tabs manually...:-)) for the time being, I will be happy!

How much are we talking about here to fix this thing?
Comment 21 Robert Knight 2008-08-26 12:36:39 UTC
> How much are we talking about here to fix this thing?

In truth, not a lot of work to get KDE 3.5.x functionality back again.  I did plan to do this for KDE 4.1 but it didn't happen because I was originally hoping to introduce improved restore of sessions since I always considered that just restoring the program and path was almost useless.  Evidently a lot of users don't feel the same way.

But please don't wait for it to happen.  The Konsole code is fairly clean and this would be a good job for a new developer.  If you haven't worked on KDE code before and you'd like to make a contribution then please get in touch.  Seasoned developers are also welcome to apply.
Comment 22 devsk 2008-08-28 06:41:13 UTC
If its not that much work, I think you should plan to add the basic restore like it was in 3.5 yourself and leave the more advanced restore to the new dev. This is one of those features which is basically pulling me back on KDE4.1. I would chip in myself but I am swamped at this time.
Comment 23 Robert Knight 2008-08-30 22:27:22 UTC
*** Bug 169982 has been marked as a duplicate of this bug. ***
Comment 24 Stefan Becker 2008-09-07 17:57:46 UTC
I finally sat down and had a look at how to implement this during the weekend. I've got the basic session management working, i.e. konsole now honors the --session option and restores the windows with position & size stored in the session file. Not much, I know, but it is a starting point.

I'll contact Robert about the modalities how we can get this to the KDE repository once it is ready.


One thing that has cost me a lot of time, pulled hairs and head-against-the-wall-banging: there seems to be a major bug in the KDE 4.1 session manager. It doesn't write restart information to $HOME/.kde/share/config/ksmserverrc for all applications, i.e. when the session gets restored at next startup some stuff will be missing. The session files are generated correctly for all applications though.

I have not been able to pinpoint this to certain applications. Basically it looks random what applications are written to ksmserverrc. Or there is some problem with the session management handling, e.g. the session information is stored but the applications already exit before ksmserverrc is generated.
Comment 25 Stefan Becker 2008-09-09 09:37:17 UTC
BTW: if anybody is interested in my changes you can grab them here:

  git clone http://chemobejk.dyndns.org/git/konsole/.git

Session state store should be complete now. Next on the agenda is to successfully restore profiles, tabs & directories from the stored information.
Comment 26 jos poortvliet 2008-09-09 11:05:03 UTC
I'm not that much into Git or build-it-yourself, but I really appreciate what you're doing, Stefan! Looking forward to seeing this code in trunk (which I DO follow).
Comment 27 Stefan Becker 2008-09-09 19:32:01 UTC
Created attachment 27341 [details]
Implement basic session management for KDE4 konsole

Success! Have fun :-)

From 93b370a0c6e1d15deb3b1db39513fcb4af2c7c2a Mon Sep 17 00:00:00 2001
From: Stefan Becker <stefan.becker@nokia.com>
Date: Tue, 9 Sep 2008 19:45:52 +0300
Subject: [PATCH] Attach sessions to views during session restore

This is the first functional version: windows, tabs, tab texts and working
directories are now saved to & restored from konsole session file. Yeah!!!

Note: For some reason profile restore doesn't seem to work correctly yet. All
      restored sessions end up with the default profile :-(
Comment 28 Robert Knight 2008-09-09 22:19:37 UTC
Hi Stefan,

Thanks.  I'll try and review it tomorrow evening.
Comment 29 Richard Hartmann 2008-09-10 03:06:13 UTC
Assuming the quirks can be ironed out soonish, will this make it into 4.1.2 or will it wait for 4.2?

Thanks Stefan!
Comment 30 Eike Hein 2008-09-10 11:11:45 UTC
Stefan, maybe you'd be interested in tackling the indirectly related bug #156919 as well while you're working on this general topic? It'd allow KPart-using apps to serialize and restore their Konsoles.
Comment 31 Stefan Becker 2008-09-10 21:50:36 UTC
Actually it seems profile save & restore is OK, now that I tested it a little bit.

But konsole's restoring of profiles at session startup seems to have some problems:
 - profiles that are currently not used by any session show up twice in the "Manage Profile" dialog
 - the "Show in Menu" status is lost
 - the "Set as Default" status is lost

Or am I seeing ghosts here?
Comment 32 devsk 2008-09-10 21:56:05 UTC
(In reply to comment #31)
> Actually it seems profile save & restore is OK, now that I tested it a little
> bit.
> 
> But konsole's restoring of profiles at session startup seems to have some
> problems:
>  - profiles that are currently not used by any session show up twice in the
> "Manage Profile" dialog
>  - the "Show in Menu" status is lost
>  - the "Set as Default" status is lost
> 
> Or am I seeing ghosts here?
> 

I have seen that happen with 4.1.0 code. It was eventually fixed in SVN, although I don't remember when.
Comment 33 Stefan Becker 2008-09-10 23:58:32 UTC
Created attachment 27358 [details]
Proposed patch for upstream merge

From d5579fdf8dab0d40c45ec9dbca071be9096955e4 Mon Sep 17 00:00:00 2001
From: Stefan Becker <stefan.becker@nokia.com>
Date: Thu, 11 Sep 2008 00:54:11 +0300
Subject: [PATCH] Restore active tab from session file

... and we also want to run() a session only once :-)

With this commit bug #152761 should be fixed. Please review & merge.
Comment 34 Stefan Becker 2008-09-11 00:23:48 UTC
Created attachment 27359 [details]
Proposed patch for upstream merge

From e764e8cd43e5a143406a0f5e7c6201004e5482c4 Mon Sep 17 00:00:00 2001
From: Stefan Becker <stefan.becker@nokia.com>
Date: Thu, 11 Sep 2008 01:21:50 +0300
Subject: [PATCH] Use tab count to determine active tab

Dooohhh... that's much easier and always restores the correct tab :-)
Comment 35 Robert Knight 2008-09-11 12:55:11 UTC
*** Bug 170221 has been marked as a duplicate of this bug. ***
Comment 36 Stefan Becker 2008-09-12 21:39:27 UTC
Created attachment 27386 [details]
Proposed patch for upstream merge

Rebased to kdebase-4.1.1.
Merged ProcessInfo refactoring from bug #156919 work.

From 07f731e3817fab7fa040cbc08ce262cda42791aa Mon Sep 17 00:00:00 2001
From: Stefan Becker <stefan.becker@nokia.com>
Date: Fri, 12 Sep 2008 22:34:16 +0300
Subject: [PATCH] Take advantage of the ProcessInfo refactoring in Session::saveS
Comment 37 Robert Knight 2008-09-15 01:17:00 UTC
Hi Stefan,

Comments on the session management part of the patch:

Functional comments on the patch:

+void MainWindow::saveGlobalProperties(KConfig* config)
+{
+    SessionManager::instance()->saveSessions(config);
+}
+
+void MainWindow::readGlobalProperties(KConfig* config)
+{
+    SessionManager::instance()->restoreSessions(config);
+}
+

An application may have multiple main windows, each window containing multiple views (TerminalDisplay instances).  Each view is associated with only one session but each session may be associated with multiple views.  This code would load all the sessions multiple times, once for each window restored.

I suggest that we simplify things to begin with by only trying to restore the first view for each session.  The process to save would be:

- Save all sessions
- For each window
-   -> Save the window properties (geometry etc.) (Use KMainWindow base class for this)
-   -> Save the association between window and session in order.  Where a session has multiple views consider only the first one.

To load:

- Load all sessions (but do not run them)
- For each window
-   -> Load the window properties (use KMainWindow base class for this)
-   -> Create one view for each session in that window, using the saved order
- Run all sessions (using session->run() )

+	// this should not happen
+	Q_ASSERT(id == 0);
+	return 0;

This assert is not needed, you have one at the start of the function with the opposite condition.  I would rewrite this sort of loop with just one exit point:

Session* foundSession = 0;
foreach(Session* session,sessions)
  if (session->sessionId() == id)
    foundSession = session;
Q_ASSERT(foundSession);
return foundSessionl


Stylistic comments:

Please try to avoid mixing formatting and functional changes in the same patch.  It makes it more difficult to examine the more important functional changes.

+    // second: all other sessions, in random order
+    // we don't want to have sessions restored that are not connected...

Please do not end comments with '...'.  It implies, to me, that the comment is only telling the reader part of what they need to know and leaving the rest as guesswork.  

+    static bool first = true;

+    SessionManager *mgr = SessionManager::instance();
+    QString profile = group.readPathEntry("Default Profile", QString());
+    Profile::Ptr pp = mgr->defaultProfile();
+    if (!profile.isEmpty()) pp = mgr->loadProfile(profile);

Use of abbreviations in variable names should generally be avoided.  They can lead to misunderstandings and consequently bugs in longer functions.  Very common abbreviations (ptr,src,dest,len,pos) may be used.

+	// session management
+	virtual void saveProperties(KConfigGroup& group);
+	virtual void readProperties(const KConfigGroup& group);
+	virtual void saveGlobalProperties(KConfig* config);
+	virtual void readGlobalProperties(KConfig* config);

When a group of virtual functions are re-implementations from a base class I find it helpful if there is a comment at the top making that clear.  In this case just delete the 'session management' comment at the line above to do that. 
Comment 38 Stefan Becker 2008-09-15 07:00:20 UTC
Hi Robert,

(In reply to comment #37)
> 
> An application may have multiple main windows, each window containing multiple
> views (TerminalDisplay instances).  Each view is associated with only one
> session but each session may be associated with multiple views.  This code
> would load all the sessions multiple times, once for each window restored.

No, it doesn't. Quote from KMainWindow::saveGlobalProperties() documentation:

  "This function is similar to saveProperties() but is only called for the very first main window, regardless how many main window are open."

Trust me, *THIS* statement I verified :-)


> +       // this should not happen
> +       Q_ASSERT(id == 0);
> +       return 0;
> 
> This assert is not needed, you have one at the start of the function with the
> opposite condition.  I would rewrite this sort of loop with just one exit
> point:

Actually the assert is correct, but you are right that it is confusing.


> Please try to avoid mixing formatting and functional changes in the same patch.
>  It makes it more difficult to examine the more important functional changes.

I know, but this is rather difficult as there are several different formatting styles in use even withing the same file :-/ So when I needed to touch existing code I decided to unify the style for that part of the code.


BTW: If you want contributors to stick to a certain style than please add the correct -*- STYLE -*- (e.g. -*- linux-c -*-) in the first line of the file so that smart editors can adjust their behavior accordingly.
Comment 39 Robert Knight 2008-09-15 12:04:49 UTC
> I know, but this is rather difficult as there are several different formatting
> styles in use even withing the same file :-/ So when I needed to touch existing 
> code I decided to unify the style for that part of the code.

Are you referring mostly to tabs vs spaces here?  Like KDE library code spaces are supposed to be used everywhere.  If so I should do a mass conversion in SVN and you could then rebase your patch on top of that.  

> No, it doesn't. Quote from KMainWindow::saveGlobalProperties() documentation:

It would help if I read that first :)
Comment 40 Stefan Becker 2008-09-15 12:17:37 UTC
Hi Robert,

(In reply to comment #39)
> 
> Are you referring mostly to tabs vs spaces here?  Like KDE library code spaces
> are supposed to be used everywhere.  If so I should do a mass conversion in SVN
> and you could then rebase your patch on top of that.  

That would help for sure.
Comment 41 Stefan Becker 2008-09-15 21:19:04 UTC
Hi Robert,

(In reply to comment #39)
> 
> Are you referring mostly to tabs vs spaces here?  Like KDE library code spaces
> are supposed to be used everywhere.  If so I should do a mass conversion in SVN
> and you could then rebase your patch on top of that.  

Adding the following lines to the end of each file will make sure that at least emacs users will stick to the same style:

/*
  Local Variables:
  mode: c++
  c-file-style: "stroustrup"
  indent-tabs-mode: nil
  tab-width: 4
  End:
*/
Comment 42 Robert Knight 2008-09-16 14:17:52 UTC
Most of the code is written with Vim, so that would be the better
format to use for
formatting hints if any.  I don't think that is necessary for the time being.

The reason for the inconsistency is that I wrote most of the code
(except for the
History.cpp and Vt102Emulation.cpp) and I changed from using tabs to spaces
mid-way through to match KDE conventions.  Since I was the only person
making changes
there wasn't a need to do a mass conversion - which I was loathe to do
since with SVN (instead of git)
because it made going through the history of the code (with svn
annotate) more difficult.  This isn't such an issue anymore
because git blame is much faster.
Comment 43 Stefan Becker 2008-09-22 20:01:49 UTC
Created attachment 27517 [details]
Proposed patch for upstream merge

* Rebased to KDE SVN commit 863413 (after whitespace cleanup)
* Reworked after review comments
Comment 44 Joe Biden 2008-09-29 16:22:19 UTC
Please, for all that is holy, implement tab management for logging back in...
Comment 45 Richard Hartmann 2008-09-29 16:33:44 UTC
As you can see, there is a patch which is seven days old & marked for review & merge. This patch would not have made it into 4.1.2 due to being too late for that, anyway. Thus, the first KDE version which can have this (very useful) feature is 4.1.3 (using KDE's version as I am not too firm on Konsole's versions and can't check, atm).

Robert, do you have any estimate on when you will be able to review Stefan's patch?
Comment 46 Eike Hein 2008-09-29 17:11:17 UTC
Please note that KDE 4.1.x releases are maintenance releases that provide only fixes, i.e. the first opportunity to ship this feature once merged is actually KDE 4.2, provided the merge happens prior to the designated feature freeze date(s) from the KDE 4.2 release schedule. The schedule can be found on the KDE TechBase wiki.
Comment 47 Richard Hartmann 2008-09-29 18:24:43 UTC
Sorry for hijacking this issue, but I assume people who follow this are interested in the answer.


I thought that rule did not apply to regressions from KDE 3 -> KDE 4? If it does, is there an appeals board or anything which can override that rule for specific features? This issue is rather annoying when using Konsole on a daily basis.
Comment 48 Andreas Pakulat 2008-09-29 18:50:49 UTC
No, there's no way to circuumvent the rule. No KDE 4.1.x release will contain new feature. Even though this is a regression from KDE3 the amount of new code already poses a problem. These things need to wait for a new minor release, because the bugfix releases usually get a lot less testing than minor releases. 

But I'm pretty sure that major distributions will backport this once its comitted to trunk.
Comment 49 Eike Hein 2008-10-01 11:18:07 UTC
> I thought that rule did not apply to regressions from KDE 3 -> KDE 4?

The reason that x.y.z releases are restricted to fixing defects is that introducing the larger amount of new code that usually goes along with adding a feature (or re-adding it, as in this case) can introduce new bugs/regressions, and users rightfully expect a bugfix release to improve their stability rather than impair it. The project has a big interest in making sure that users don't fear upgrading to a bugfix release for whatever else might be in it.

Trust me, developers would often love to smuggle new features (or old ones) into bugfix releases; it takes discipline to hold back things, too, but it's very important. 
Comment 50 Richard Hartmann 2008-10-01 12:22:03 UTC
I agree with what you say & it is a very good thing to do.

I just thought that it did not apply to regressions, thanks for clarifying that :)
Comment 51 Quantum Five 2008-10-03 01:23:47 UTC
(In reply to comment #46, #48 and #49)

To: Eike Hein, Andreas Pakulat

> Please note that KDE 4.1.x releases are maintenance releases that provide only fixes, [...]
> No, there's no way to circuumvent the rule.
> [...] to ship this feature once merged is actually KDE 4.2, provided the merge happens prior to the designated feature freeze date(s) from the KDE 4.2 release schedule.


I am often convinced that KDE 4 management team (if something like that actually exists) lives in a cave on the Moon without any grip to the reality and completely oblivious to anything surrounding them.

So allow me to tell it straight to you - this part should be flashing in bold capital huge red letters, maybe that way you will GET IT !!!

As opposed to widgets and other plasma gimmicks, session management in konsole KDE 4.1.x is **CRUCIAL** for most of us end-users. Without it the KDE 4.1.x is pretty much **useless** to us and we'll throw the whole lot to the bin and use KDE 3 instead. Clear enough ?

Do you understand what CRUCIAL word mean ? Do you understand what words "MUST HAVE IT BACK NOW" mean ?

For us this omission represents a major **BUG** which needs to be fixed ASAP. Allow me to be blunt here but what it means to you it's irrelevant really because it is **our** interpretation and request that should rule, not yours.

And we really don't care about, or want to hear your excuses for not getting it into the next bug fixing release. What we want is action ! Now !

So you have two options: either continue to be ignorant and offer us your same lame excuses and alienate all potential KDE 4 users or wake up for God's sake and do the right thing: fix it NOW.

We grew tired of seeing this sheer stupidity over and over again: new features are added and given high priority, features that will be used by very few of us - if ever, while **BASIC FUNCTIONALITY** - let me repeat this: BASIC FUNCTIONALITY (!!!!) that 99% end users need it **NOW** is missing and the fix is promised to land once in a blue moon.

> [...] it takes discipline to hold back things, [...]

No, it takes stupidity not discipline !

DOH !!!!
Comment 52 devsk 2008-10-03 01:45:06 UTC
(In reply to comment #51)

> No, it takes stupidity not discipline !
> 
> DOH !!!!
> 

Oh C'mon! You can reason with people here! So what if Stefan Becker spent his weekend generating the patch. It doesn't make konsole flash or wobble or do splits. Something else does! So, cut it!

Ok, may be I was wrong about the splits...:)
Comment 53 Greg Ennis 2008-10-03 02:13:42 UTC
To all,

If two cents are worth anything.  Reality Guy has a point.  We have delayed any conversion to KDE 4 because of this problem.  The first system we used with Fedora 9 was a flop because of this issue.  At the same time I genuinely appreciate what the developers are doing.  We will just wait for the enhancements .... but without holding our breath.

Greg Ennis
Comment 54 Joe Biden 2008-10-03 03:07:56 UTC
> So you have two options: either continue to be ignorant and offer us your same
> lame excuses and alienate all potential KDE 4 users or wake up for God's sake
> and do the right thing: fix it NOW.

Honestly, it's critical to fix this, but if they have a good reason for not letting it go into maintenance releases, then I guess we'll have to forgo moving to KDE 4.1.[012] at my work, as this was a necessary feature.

I can live without it, but it's certainly annoying. I certainly don't think calling anyone out on this issue, cowboy-style will inspire them to change their minds though.
Comment 55 Stefan Becker 2008-10-03 07:47:15 UTC
I can understand Reality Guy's (and many others) frustration about this. Thats the reason why the patch exists :-)

I just checked the trunk in WebSVN and the patch hasn't been applied yet. According to

 <http://techbase.kde.org/Schedules/KDE4/4.2_Release_Schedule>

the freeze for 4.2 is on the 19th of October and that's just 2 weeks from now. No pressure, but if the patch doesn't make it then I think comment #51 will be one of the nicest things you'll hear about this...
Comment 56 Andreas Pakulat 2008-10-03 11:31:05 UTC
Two notes regarding Stefans last post:

a) thats the soft-freeze, which means new features need to be started by then, they don't have to be finished. And I'd consider this patch attached here starting the feature, hence there's time until november 17th

b) If Robert doesn't find time to review it, I'll have a look myself and test it a bit. If it works well I'll make sure it'll be in before November 17th.
Comment 57 Eike Hein 2008-10-03 11:36:38 UTC
The point is that incorporating additional functionality in a maintenance release always potentially risks further regressions, and making deployments distrust KDE maintenance releases is a sure-fire way to alienate users just as well. Deciding whether the incorporation of an additional feature is "going to be fine this time, nothing will happen!!" on a case-by-case basis is both frivolous, and also requires resources that are not available either inside or outside the project (it would require starting to do betas and release candidates for maintenance releases, and a longer cycle than a month - sounds like the major releases we do every six month, anyone?).

And no, insults really will not motivate anyone to cater to your needs either.
Comment 58 S. Burmeister 2008-10-03 12:03:40 UTC
> I am often convinced that KDE 4 management team (if something like that
> actually exists) lives in a cave on the Moon without any grip to the reality
> and completely oblivious to anything surrounding them.

The approach to not include new features into maintenance releases is very common, so there must be a huge cave.

> So allow me to tell it straight to you - this part should be flashing in bold
> capital huge red letters, maybe that way you will GET IT !!!

This is the point where you already lost your audience in the developers camp, because it shows that what follows is polemicand just trolling.

> As opposed to widgets and other plasma gimmicks, session management in konsole
> KDE 4.1.x is **CRUCIAL** for most of us end-users. Without it the KDE 4.1.x is
> pretty much **useless** to us and we'll throw the whole lot to the bin and use
> KDE 3 instead. Clear enough ?

"most of end-users [...] we will throw [...]". You do not have any evidence that you know what most end-users think is crucial, emphasising your ignorance with stars and capital letters really makes you look bad, using pluralis majestatis, makes you even look arrogant.

> Do you understand what CRUCIAL word mean ? Do you understand what words "MUST
> HAVE IT BACK NOW" mean ?

You really like to show off your bad traits, do you?

> For us this omission represents a major **BUG** which needs to be fixed ASAP.
> Allow me to be blunt here but what it means to you it's irrelevant really
> because it is **our** interpretation and request that should rule, not yours.

Code rules, nothing else. And since you seem to have contributed very little code to the project, you have as much as that to decide. You would really have to contribute a lot to make up for your attitude.

> And we really don't care about, or want to hear your excuses for not getting it
> into the next bug fixing release. What we want is action ! Now !
> 
> So you have two options: either continue to be ignorant and offer us your same
> lame excuses and alienate all potential KDE 4 users or wake up for God's sake
> and do the right thing: fix it NOW.
> 
> We grew tired of seeing this sheer stupidity over and over again: new features
> are added and given high priority, features that will be used by very few of us
> - if ever, while **BASIC FUNCTIONALITY** - let me repeat this: BASIC
> FUNCTIONALITY (!!!!) that 99% end users need it **NOW** is missing and the fix
> is promised to land once in a blue moon.
> 
> > [...] it takes discipline to hold back things, [...]
> 
> No, it takes stupidity not discipline !
> 
> DOH !!!!

Let me show you why your post is completely useless: Normal users, e.g. those coming from windows, do not like to touch anything that looks like a command line, let alone using session management in their kosnole, so those users that do, seem to have some deeper knowledge and interest, might even know about coding a bit. If the latter is the majority and this was crucial for them, it would be very likely that one of them would have submitted a patch during the 4.0.x time already. Didn't happen.

Trying to claim that 99% of the users need konsole _and_ its session management is just ridiculous, ~300 votes speak for themselves. There is a need for this functionality, which nobody ever doubted, but if the majority would need it crucially, there would be other numbers, check bugzilla for bugs the majority really hates.

Most people use some distro. Distros include patches, even unreleased ones if they think they are needed. They even create patches and submit them upstream, if they think they are  needed. Didn't happen(, yet).

Smart users remember that konsole3 worked for them and just continue to use it, even if they run KDE4.

Smart users know that calling developers and the KDE team stupid disqualifies them as people to care about. Even if you were right, your attitude is unbearable.

If this functionality is available at some point it is due to the effort of Stefan Becker and not because of people like you that don't code but just troll. If I would shout at you that 1+1=2 and call you stupid at the same time, even though I am right, you would just laugh about me, that's what people do when reading your trolling.
Comment 59 Robert Knight 2008-10-03 12:48:04 UTC
Hi Stefan,

Thank-you for being patient.  I reviewed and did some light testing on the latest patch last night but didn't have a change to apply it.  There are some small changes I will likely make at the weekend but I have committed it to Subversion as is for the time being.  I can then make any further changes in my own time.  Or alternatively you can get a KDE subversion account if you don't already have one.

====

SVN revision 867323.

Initial implementation of session management in Konsole.
Patch from Stefan Becker <stefan.becker@nokia.com>

Thank-you very much Stefan!

CCBUG: 152761

====

@ Everyone, especially distributors regarding backporting

I agree with Richard.  Regressions from KDE 3 -> 4 can be backported as long as they do not add any UI changes and they have sufficient testing.  
Comment 60 Robert Knight 2008-10-03 12:50:45 UTC
Also regarding backporting, this patch is quite large (~30KB) so I would recommend that distributors do not apply it verbatim to 4.1 right away.  I will try to find time at the weekend to test it against KDE 4.1 but I'd welcome help from anyone who has more free time.
Comment 61 Stefan Becker 2008-10-03 14:02:09 UTC
I've created a backport request for Fedora 9/10:

 <https://bugzilla.redhat.com/show_bug.cgi?id=465451>
Comment 62 Robert Knight 2008-10-03 14:34:23 UTC
Just to clarify my comments above.  I haven't tested this patch against KDE 4.1 at all yet.  
Comment 63 Will Stephenson 2008-10-03 17:24:47 UTC
Sorry folks, r867459 (with the committed patch) crashes reproducibly on logout.

The Application: Konsole (konsole), signal SIGABRT
[?1034h[Thread debugging using libthread_db enabled]
0x00007f3d872a4841 in nanosleep () from /lib64/libc.so.6
[Current thread is 1 (Thread 0x7f3d8c686750 (LWP 13903))]

Thread 2 (Thread 0x7f3d8191e950 (LWP 13904)):
#0  0x00007f3d872cf012 in select () from /lib64/libc.so.6
#1  0x00007f3d88019296 in QProcessManager::run (this=0x63dc90) at io/qprocess_unix.cpp:307
#2  0x00007f3d87f50f32 in QThreadPrivate::start (arg=0x63dc90) at thread/qthread_unix.cpp:191
#3  0x00007f3d87cdd070 in start_thread () from /lib64/libpthread.so.0
#4  0x00007f3d872d5acd in clone () from /lib64/libc.so.6
#5  0x0000000000000000 in ?? ()

Thread 1 (Thread 0x7f3d8c686750 (LWP 13903)):
[KCrash Handler]
#5  0x00007f3d87232745 in raise () from /lib64/libc.so.6
#6  0x00007f3d87233d33 in abort () from /lib64/libc.so.6
#7  0x00007f3d87f49265 in qt_message_output (msgType=QtFatalMsg, buf=<value optimized out>) at global/qglobal.cpp:2108
#8  0x00007f3d87f493ad in qFatal (msg=<value optimized out>) at global/qglobal.cpp:2309
#9  0x00007f3d87f4941a in qt_assert (assertion=<value optimized out>, file=<value optimized out>, line=-1) at global/qglobal.cpp:1878
#10 0x00007f3d8c1ddd6e in KSharedPtr<Konsole::Profile>::operator-> (this=0x6efb20) at /space/kde/installs/trunk/include/KDE/../ksharedptr.h:116
#11 0x00007f3d8c2567c1 in Konsole::MainWindow::saveProperties (this=0x6efac0, group=@0x7fff946bcd90) at /space/kde/sources/trunk/KDE/kdebase/apps/konsole/src/MainWindow.cpp:385
#12 0x00007f3d8bc2494e in KMainWindow::savePropertiesInternal (this=0x6efac0, config=0xb44ec0, number=1) at /space/kde/sources/trunk/KDE/kdelibs/kdeui/widgets/kmainwindow.cpp:620
#13 0x00007f3d8bc280e8 in KMWSessionManager::saveState (this=0x6f5680) at /space/kde/sources/trunk/KDE/kdelibs/kdeui/widgets/kmainwindow.cpp:150
#14 0x00007f3d8bb622ee in KApplication::saveState (this=0x7fff946be1f0, sm=@0x66d7a0) at /space/kde/sources/trunk/KDE/kdelibs/kdeui/kernel/kapplication.cpp:819
#15 0x00007f3d89f4bb46 in sm_performSaveYourself (smd=0x679f90) at kernel/qapplication_x11.cpp:5116
#16 0x00007f3d89f4beda in sm_saveYourselfCallback (smcConn=<value optimized out>, clientData=0x364f, saveType=-2024437296, shutdown=<value optimized out>, interactStyle=-1)
    at kernel/qapplication_x11.cpp:5065
#17 0x00007f3d8993c737 in _SmcProcessMessage () from /usr/lib64/libSM.so.6
#18 0x00007f3d8972b90e in IceProcessMessages () from /usr/lib64/libICE.so.6
#19 0x00007f3d89f41479 in QSmSocketReceiver::qt_metacall (this=0x6b87f0, _c=QMetaObject::InvokeMetaMethod, _id=0, _a=0x7fff946bd8f0) at .moc/release-shared/qapplication_x11.moc:64
#20 0x00007f3d8804d3d4 in QMetaObject::activate (sender=0x6bebf0, from_signal_index=<value optimized out>, to_signal_index=4, argv=0xffffffffffffffff) at kernel/qobject.cpp:3037
#21 0x00007f3d8808400e in QSocketNotifier::activated (this=0x364f, _t1=18) at .moc/release-shared/moc_qsocketnotifier.cpp:81
#22 0x00007f3d88052f23 in QSocketNotifier::event (this=0x6bebf0, e=0x7fff946bdef0) at kernel/qsocketnotifier.cpp:326
#23 0x00007f3d89ee8cad in QApplicationPrivate::notify_helper (this=0x62e070, receiver=0x6bebf0, e=0x7fff946bdef0) at kernel/qapplication.cpp:3809
#24 0x00007f3d89ef0a7a in QApplication::notify (this=0x7fff946be1f0, receiver=0x6bebf0, e=0x7fff946bdef0) at kernel/qapplication.cpp:3774
#25 0x00007f3d8bb627c0 in KApplication::notify (this=0x7fff946be1f0, receiver=0x6bebf0, event=0x7fff946bdef0) at /space/kde/sources/trunk/KDE/kdelibs/kdeui/kernel/kapplication.cpp:307
#26 0x00007f3d88039011 in QCoreApplication::notifyInternal (this=0x7fff946be1f0, receiver=0x6bebf0, event=0x7fff946bdef0) at kernel/qcoreapplication.cpp:593
#27 0x00007f3d880616e9 in socketNotifierSourceDispatch (source=0x634ea0) at kernel/qcoreapplication.h:215
#28 0x00007f3d85d1507b in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#29 0x00007f3d85d1885d in ?? () from /usr/lib64/libglib-2.0.so.0
#30 0x00007f3d85d18a1b in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0
#31 0x00007f3d8806146f in QEventDispatcherGlib::processEvents (this=0x62dfd0, flags=<value optimized out>) at kernel/qeventdispatcher_glib.cpp:325
#32 0x00007f3d89f7944f in QGuiEventDispatcherGlib::processEvents (this=0x364f, flags=<value optimized out>) at kernel/qguieventdispatcher_glib.cpp:204
#33 0x00007f3d88037922 in QEventLoop::processEvents (this=<value optimized out>, flags={i = -1804869360}) at kernel/qeventloop.cpp:149
#34 0x00007f3d88037aad in QEventLoop::exec (this=0x7fff946be150, flags={i = -1804869280}) at kernel/qeventloop.cpp:200
#35 0x00007f3d88039f7d in QCoreApplication::exec () at kernel/qcoreapplication.cpp:851
#36 0x00007f3d8c25a8c4 in kdemain (argc=1, argv=0x7fff946be468) at /space/kde/sources/trunk/KDE/kdebase/apps/konsole/src/main.cpp:97
#37 0x00000000004008f7 in main (argc=1, argv=0x7fff946be468) at /space/kde/builds/trunk/KDE/kdebase/apps/konsole/src/konsole_dummy.cpp:3

Comment 64 Will Stephenson 2008-10-03 17:34:09 UTC
I tried moving apps/konsole and konsolerc aside; makes no difference.  It also crashes whether I have more than one tab open or not.
Comment 65 Robert Knight 2008-10-03 17:45:24 UTC
Hi Will,

Its a simple null pointer check missing in the case where a window is opened using the default profile.  Fix committed (rv. 867477)
Comment 66 Will Stephenson 2008-10-03 18:00:51 UTC
That was quick!  The patch seems fine now for my use case, which is: default profile, multiple tabs, various working directories and tab labels.
Comment 67 Richard Hartmann 2008-10-03 20:09:39 UTC
Stefan: Thanks for the work.

Robert: Thanks for the feedback

Burrmeister: While you can use KDE 3 apps with KDE 4, it is a bad crutch, at best. Personally, I try to avoid pulling old 'cruft' into KDE 4.

Troll: While you do have a point inasmuch that this feature is crucial, your behavior is absolutely unacceptible. Period.
I am using KDE 4 on one of the four computers which I use on a regular basis. The lack of Konsole's session management is one of the main reasons for this. I don't bitch and moan about this, though.
Comment 68 Mike 2008-10-04 14:52:45 UTC
Just for your information, I updated to HEAD yesterday and when I started KDE, Konsole started up without any tabs open at all.  The grey window was there but with no tab.  Clicking the menu and opening a new tab has fixed it now but there is a problem with no/invalid session data.
Comment 69 Kurt Hindenburg 2008-10-04 18:33:42 UTC
Yes, I have the same thing as #68 upon startup.
Comment 70 Stefan Becker 2008-10-05 17:34:42 UTC
(In reply to comment #68)
> Just for your information, I updated to HEAD yesterday and when I started KDE,
> Konsole started up without any tabs open at all.  The grey window was there but
> with no tab.  Clicking the menu and opening a new tab has fixed it now but
> there is a problem with no/invalid session data.

Just backported the patch to the F9 kdebase-4.1.1 RPM and I can't reproduce the problem.

Comment 71 Will Stephenson 2008-10-05 22:15:57 UTC
Stefan: To reproduce, start a session with the patch, but restoring session saved without the patch.

Could you attach the patch to the 4.1 branch here, or put it in an svn branch?

Comment 72 Kevin Kofler 2008-10-05 22:17:25 UTC
He posted the patch against 4.1 to the RH/Fedora Bugzilla:
https://bugzilla.redhat.com/attachment.cgi?id=319498
Comment 73 Robert Knight 2008-10-06 00:17:07 UTC
Hi Stefan,

Please attach backport patches to this thread before, or at least in addition to, distribution-specific bug trackers.  This patch will be of interest to Kubuntu and Suse as well.  Plus as we have seen, the version of the patch I applied caused some regressions which needed minor code changes to fix.
Comment 74 Kevin Kofler 2008-10-06 00:24:31 UTC
I forgot to mention the patch description:
> KDE SVN trunk revisions 867154 to 867477 changes backported to 4.1.1
So this has your fixes up to revision 867477 included too.

And don't worry, I'll keep you in the loop about any patches for this issue uploaded to the Fedora bug tracker. (That said, I'd also prefer that Stefan Becker uploads them here too.)
Comment 75 Stefan Becker 2008-10-06 05:51:57 UTC
Created attachment 27708 [details]
KDE SVN trunk revisions 867154 to 867477 changes backported to 4.1.1

Here you go...
Comment 76 Stefan Becker 2008-10-06 07:16:07 UTC
Created attachment 27709 [details]
Fix restore from invalid session data

This should fix the problem when restoring from a session saved by KDE3. Can you please try this?
Comment 77 Robert Knight 2008-10-06 15:00:19 UTC
Hi Stefan,

Comments on #76:

We should print a more friendly/helpful message to the terminal if there is invalid session data.  eg. "Unable to restore saved tabs.  Possibly unsupported KDE 3 session data.".

It should also be kWarning() rather than kError() and should, I think, also be translated with i18n(string) because users will see it with release builds.

I suggest calling the config entry 'ActiveTabIndex' instead of just 'Active' to make the file easier to read. 

Unless you need functionality which is specific to the TerminalDisplay class, and it looks like you don't, then it would be better to make 'display' a QWidget*.
Comment 78 Stefan Becker 2008-10-06 17:18:22 UTC
Created attachment 27718 [details]
Fix restore from invalid session data

Updated after review comments.
Comment 79 Pino Toscano 2008-10-25 12:42:03 UTC
*** Bug 173455 has been marked as a duplicate of this bug. ***
Comment 80 Kevin Kofler 2008-10-29 18:38:18 UTC
What happened to this update/followup?
http://bugs.kde.org/attachment.cgi?id=27718
I don't see this committed to the trunk yet (the code in trunk still uses "Active", not "ActiveTabIndex", and it doesn't have the KDE3 session part), can you please commit it? Or is there a reason you didn't commit it?
Comment 81 Jonathan Thomas 2009-01-02 20:52:25 UTC
*** Bug 179378 has been marked as a duplicate of this bug. ***
Comment 82 Juan Ignacio Pumarino 2009-02-20 23:52:35 UTC
Hi,

When was this resolved? Is it coming on 4.3? Thanks in advance.
Comment 83 S. Burmeister 2009-02-20 23:58:33 UTC
Reading the comments would help. this is resolved for KDE 4.1 and .2 etc.
Comment 84 Juan Ignacio Pumarino 2009-02-21 00:03:13 UTC
S.: But tabs can't yet be saved, right?
Comment 85 jos poortvliet 2009-02-21 11:31:53 UTC
@juan: you mean to ask if it does restore tabs with names and locations at startup? Works fine here.... At least on 4.2 :D
Comment 86 MixMaster 2009-05-11 23:52:12 UTC
How i can save my profile with all opened tabs and later restore it? I cant find "save profile" in my kde 4.2.2 konsole.
Thanks in advance
Comment 87 Robert Knight 2009-05-12 01:55:11 UTC
It happens automatically if you log out and back in again.  There is no way to do it manually at the moment.
Comment 88 Stefan Becker 2009-05-12 06:25:00 UTC
... or use "Save Session" from the "Leave" tab in the Kickoff menu.

Anyway: konsole doesn't have it's own session management, it's all handled by KDE. When KDE saves the session information, either automatically or when asked by the user, then konsole provides its session information.
Comment 89 Robert Knight 2009-05-12 10:37:34 UTC
> Anyway: konsole doesn't have it's own session management,
> it's all handled by KDE.
> When KDE saves the session information, either automatically
> or when asked by the user, then konsole provides its session information.

That is an implementation detail and something which users shouldn't need to know.