--===============2125930201783169457== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Sept. 29, 2012, 9:20 a.m., David Faure wrote: > > konqueror/src/konqsessionmanager.cpp, line 450 > > > > > > Doesn't this lose ItemIsSelectable? I guess item->flags() | Qt::Ite= mIsUserCheckable would be better. I did this on purpose. I do not want the items to be selectable. I just wan= t them to be checkable. > On Sept. 29, 2012, 9:20 a.m., David Faure wrote: > > konqueror/src/konqsessionmanager.cpp, line 339 > > > > > > 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 clas= s, and an ugly qobject_cast... > > = > > 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 reusabili= ty. (and readability). > > = > > Thanks. Good point. I will redo it that way then. - Dawit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106503/#review19563 ----------------------------------------------------------- On Sept. 28, 2012, 4:45 p.m., Dawit Alemayehu wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106503/ > ----------------------------------------------------------- > = > (Updated Sept. 28, 2012, 4:45 p.m.) > = > = > Review request for KDE Base Apps and David Faure. > = > = > Description > ------- > = > The attached patch fixes one of those pet peeve bugs that infurate me fro= m 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. > = > = > This addresses bug 260282. > http://bugs.kde.org/show_bug.cgi?id=3D260282 > = > = > Diffs > ----- > = > konqueror/src/konqsessionmanager.h ee629e4 = > konqueror/src/konqsessionmanager.cpp 68a003f = > = > Diff: http://git.reviewboard.kde.org/r/106503/diff/ > = > = > Testing > ------- > = > * Unselected sessions should not be restored. > * If all available sessions are selected (the default), the behavior shou= ld remain the same as it is today. > * If all available sessions are unselected, disable the "Restore Session"= button. > = > = > Screenshots > ----------- > = > old restore dialog > http://git.reviewboard.kde.org/r/106503/s/729/ > new restore dialog > http://git.reviewboard.kde.org/r/106503/s/731/ > new restore dialog v2 > http://git.reviewboard.kde.org/r/106503/s/739/ > = > = > Thanks, > = > Dawit Alemayehu > = > --===============2125930201783169457== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/106503/

On September 29th, 2012, 9:20 a.m., David F= aure wrote:

= = =
konqueror/src/konqsessionmanager.cpp (Diff revision 3) 

   
static int showRestoreSessionDialog(QObject* object, const SessionH=
ash& sessionList, QStringList* discardedSessionList)


  

 

  

    
    

    339
    
    QPointer<KDialog><=
/span> dialog =3D 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 qo=
bject_cast<window()>...

If you split this up into a proper class, then it can have its own slots an=
d 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 F=
aure wrote:

 

  =









 =

 =




 

 
  

   
    konqueror/src/konqsessionmanager.cpp
    

     (Diff revision 3)

    
   
  

 

 
  


   

   
static int showRestoreSessionDialog(QObject* object, const SessionH=
ash& sessionList, QStringList* discardedSessionList)


  

 

  

    
    

    450
    
                item<=
span class=3D"o">->setFlags(Qt::ItemIsEnabled | Qt::ItemIsUserCheckabl=
e);

  


 




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

 









I did this on purpose. I do not want the items to be selectable. I j=
ust 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.








Descripti=
on 

 

 

  
   
The attached patch fixes one of those pet peeve bugs that in=
furate me from time to time by allowing me to unselect the sessions I do no=
t want to be restored when Konqueror's restore session dialog pops up.<=
/pre>
  

 






Testing <=
/h1>


 

  
   
* 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 Sessi=
on" button.

  		
 








 Bugs: 


 260282







Diffs=
 


    

 
  • konqueror/src/konqsessionmanager.h (ee629e=
4)
    • 

 
  • konqueror/src/konqsessionmanager.cpp (68a0=
03f)
    • 





View Diff





Screensho=
ts 





 3D"old

 3D"new=

 3D"new=





  		
 











  

 



--===============2125930201783169457==--