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

On September 29th, 2012, 9:20 a.m., David Faure wrote:

konqueror/src/konqsessionmanager.cpp (Diff revision 3)
		static int showRestoreSessionDialog(QObject* object, const SessionHash& sessionList, QStringList* discardedSessionList)
		339	QPointer<KDialog> dialog = new KDialog(0, Qt::Dialog);

This code would be better with a KDialog subclass, rather than with all the code in a function -- and then you need a slot in the calling class, and an ugly qobject_cast<window()>...

If you split this up into a proper class, then it can have its own slots and member variables, and this will increase modularity and reusability. (and readability).

Thanks.

Good point. I will redo it that way then.

On September 29th, 2012, 9:20 a.m., David Faure wrote:

konqueror/src/konqsessionmanager.cpp (Diff revision 3)
		static int showRestoreSessionDialog(QObject* object, const SessionHash& sessionList, QStringList* discardedSessionList)
		450	item->setFlags(Qt::ItemIsEnabled | Qt::ItemIsUserCheckable);

Doesn't this lose ItemIsSelectable? I guess item->flags() | Qt::ItemIsUserCheckable would be better.

I did this on purpose. I do not want the items to be selectable. I just want them to be checkable.

- Dawit

On September 28th, 2012, 4:45 p.m., Dawit Alemayehu wrote:

Review request for KDE Base Apps and David Faure. By Dawit Alemayehu. Updated Sept. 28, 2012, 4:45 p.m. Description The attached patch fixes one of those pet peeve bugs that infurate me from time to time by allowing me to unselect the sessions I do not want to be restored when Konqueror's restore session dialog pops up. Testing * Unselected sessions should not be restored. * If all available sessions are selected (the default), the behavior should remain the same as it is today. * If all available sessions are unselected, disable the "Restore Session" button. Bugs: 260282 Diffs konqueror/src/konqsessionmanager.h (ee629e4) konqueror/src/konqsessionmanager.cpp (68a003f) View Diff Screenshots