--===============0534491195== Content-Type: multipart/alternative; boundary="===============7081252574658450110==" --===============7081252574658450110== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/4528/#review6390 ----------------------------------------------------------- Mostly looks good to me. A few minor comments. trunk/KDE/kdebase/apps/konsole/src/RenameTabsDialog.cpp The standard accessor naming is setProperty(), and property() so this w= ould be named tabTitleText(). = (I suspect that might not be followed in some parts of code, but I thin= k that was the standard in most new code. Same as Qt in other words). trunk/KDE/kdebase/apps/konsole/src/SessionController.cpp I would recommend std::auto_ptr or QScopedPointer in favor of manual de= letion. - Robert On 2010-07-06 15:05:44, Kurt Hindenburg wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/4528/ > ----------------------------------------------------------- > = > (Updated 2010-07-06 15:05:44) > = > = > Review request for Konsole. > = > = > Summary > ------- > = > The current rename tab is very bland and non-user friendly. This new dia= log mimics the Edit Profile-> Tab Titles section. > = > = > This addresses bug 228129. > https://bugs.kde.org/show_bug.cgi?id=3D228129 > = > = > Diffs > ----- > = > trunk/KDE/kdebase/apps/konsole/src/CMakeLists.txt 1146398 = > trunk/KDE/kdebase/apps/konsole/src/RenameTabsDialog.h PRE-CREATION = > trunk/KDE/kdebase/apps/konsole/src/RenameTabsDialog.cpp PRE-CREATION = > trunk/KDE/kdebase/apps/konsole/src/RenameTabsDialog.ui PRE-CREATION = > trunk/KDE/kdebase/apps/konsole/src/SessionController.cpp 1146398 = > = > Diff: http://reviewboard.kde.org/r/4528/diff > = > = > Testing > ------- > = > trunk > = > = > Thanks, > = > Kurt > = > --===============7081252574658450110== 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://reviewboard.kde= .org/r/4528/ 

Mostly looks good to me.  A few minor comments.

= =
trunk/KDE/kdebase/= apps/konsole/src/RenameTabsDialog.cpp (Diff revision 1) 

   
void RenameTabsDialog:=
:setRemoteTabTitleText(const QString& text)


  

 

  

    
    
<=
/pre>

    64
    
<=
span class=3D"n">QString RenameTabsDialog::getTabTitleText() const

  


 





The standard accessor naming is setProperty=
(), and property() so this would be named tabTitleText().

(I suspect that might not be followed in some parts of code, but I think th=
at was the standard in most new code.  Same as Qt in other words).
















 =

 =




 

 
  

   
    trunk/KDE/kdebase=
/apps/konsole/src/SessionController.cpp
    

     (Diff revision 1)

    
   
  

 

 
  


   
void SessionController=
::editCurrentProfile()


  

 

  

    
    
<=
/pre>

    569
    
 =
   delete dialog;

  


 





I would recommend std::auto_ptr or QScopedP=
ointer in favor of manual deletion.









- Robert






On July 6th, 2010, 3:05 p.m., Kurt Hindenburg wrote:







 

  


Review request for Konsole.


By Kurt Hindenburg.




Updated 2010-07-06 15:05:44






Descripti=
on 



 

  
   
The current rename tab is very bla=
nd and non-user friendly.  This new dialog mimics the Edit Profile-> Tab=
 Titles section.

  		
 






Testing <=
/h1>


 

  
   
trunk

  		
 








 Bugs: 


 228129







Diffs=
 


    

 
  • trunk/KDE/kdebase/apps/konsole/src/CMakeLists.txt (1146398)
    • 

 
  • trunk/KDE/kdebase/apps/konsole/src/RenameTabsDialog.h (PRE-CREATION)
    • 

 
  • trunk/KDE/kdebase/apps/konsole/src/RenameTabsDialog.cpp (PRE-CREATION)
    • 

 
  • trunk/KDE/kdebase/apps/konsole/src/RenameTabsDialog.ui (PRE-CREATION)
    • 

 
  • trunk/KDE/kdebase/apps/konsole/src/SessionController.cpp (1146398)
    • 





View Diff





  		
 











  

 



--===============7081252574658450110==--

--===============0534491195==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
konsole-devel mailing list
konsole-devel@kde.org
https://mail.kde.org/mailman/listinfo/konsole-devel

--===============0534491195==--