From kde-panel-devel Tue Aug 28 14:56:31 2012 From: "David Edmundson" Date: Tue, 28 Aug 2012 14:56:31 +0000 To: kde-panel-devel Subject: Re: Review Request: Dictionary Runner Message-Id: <20120828145631.8723.22473 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=134616581808218 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============4996122883384326407==" --===============4996122883384326407== Content-Type: multipart/alternative; boundary="===============3813008473124798931==" --===============3813008473124798931== 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://git.reviewboard.kde.org/r/106068/#review18164 ----------------------------------------------------------- Ship it! Code was generally really good, minor comments below. One somewhat importan= t. runners/dictionary/dictionarymatchengine.h *important* = This is not a valid license header. Copy and paste the correct one from= somewhere else. runners/dictionary/dictionarymatchengine.cpp License runners/dictionary/dictionaryrunner.h License...etc for all other files. runners/dictionary/dictionaryrunner_config.cpp Use KLineEdit unless there's a good reason not to. runners/dictionary/dictionaryrunner_config.cpp Would be better to get this i18nc appearing only once. = = (a static method somewhere perhaps) = currently if a translator translates it differently in one of the 3 pla= ces, it breaks. - David Edmundson On Aug. 18, 2012, 3:57 a.m., Jason A. Donenfeld wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106068/ > ----------------------------------------------------------- > = > (Updated Aug. 18, 2012, 3:57 a.m.) > = > = > Review request for Plasma, Jason A. Donenfeld and Aaron J. Seigo. > = > = > Description > ------- > = > Jason's dictionary runner plugin for krunner. > = > = > Diffs > ----- > = > runners/CMakeLists.txt f487563c58b5bacc5e49b9e1a0f6e306956bbf7f = > runners/dictionary/CMakeLists.txt PRE-CREATION = > runners/dictionary/Messages.sh PRE-CREATION = > runners/dictionary/dictionarymatchengine.h PRE-CREATION = > runners/dictionary/dictionarymatchengine.cpp PRE-CREATION = > runners/dictionary/dictionaryrunner.h PRE-CREATION = > runners/dictionary/dictionaryrunner.cpp PRE-CREATION = > runners/dictionary/dictionaryrunner_config.h PRE-CREATION = > runners/dictionary/dictionaryrunner_config.cpp PRE-CREATION = > runners/dictionary/plasma-runner-dictionary.desktop PRE-CREATION = > runners/dictionary/plasma-runner-dictionary_config.desktop PRE-CREATION = > = > Diff: http://git.reviewboard.kde.org/r/106068/diff/ > = > = > Testing > ------- > = > Works well under various different loads. > = > = > Screenshots > ----------- > = > The Runner in Action > http://git.reviewboard.kde.org/r/106068/s/679/ > = > = > Thanks, > = > Jason A. Donenfeld > = > --===============3813008473124798931== 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/106068/

Ship it!

Code was g=
enerally really good, minor comments below. One somewhat important.

= =
runners/dictionary/dictionarymatchengine.h (Diff revision 1) 


  

 

  

    
    

    2
    
 * Copyright (C) 2010, 2012=
 Jason A. Donenfeld <Jason@zx2c4.com>

  


 





*important*

This is not a valid license header. Copy and paste the correct one from som=
ewhere else.

















 =

 =




 

 
  

   
    runners/dictionary/dictionarymatchengine.cpp
    

     (Diff revision 1)

    
   
  

 

 
  


   


  

 

  

    
    

    2
    
 * Copyright (C) 2010, 2012=
 Jason A. Donenfeld <Jason@zx2c4.com>

  


 





License

















 =

 =




 

 
  

   
    runners/dictionary/dictionaryrunner.h
    

     (Diff revision 1)

    
   
  

 

 
  


   


  

 

  

    
    

    2
    
 * Copyright (C) 2010, 2012=
 Jason A. Donenfeld <Jason@zx2c4.com>

  


 





License...etc for all other files.

















 =

 =




 

 
  

   
    runners/dictionary/dictionaryrunner_config.cpp
    

     (Diff revision 1)

    
   
  

 

 
  


   

   
DictionaryRunnerConfig::DictionaryRunnerConfig(QWidget* parent, con=
st QVariantList& args) :


  

 

  

    
    

    16
    
	m=
_triggerWord =3D new QLineEdit;

  


 





Use KLineEdit unless there's a good reason not to.

















 =

 =




 

 
  

   
    runners/dictionary/dictionaryrunner_config.cpp
    

     (Diff revision 1)

    
   
  

 

 
  


   

   
DictionaryRunnerConfig::DictionaryRunnerConfig(QWidget* parent, con=
st QVariantList& args) :


  

 

  

    
    

    29
    
	m=
_triggerWord->setText<=
/span>(grp.readEntry(CONFIG_TRIGGERWORD, i18nc("Trig=
ger word before word to define", "define")));

  


 





Would be better to get this i18nc appearing only once. =


(a static method somewhere perhaps)

currently if a translator translates it differently in one of the 3 places,=
 it breaks.









- David






On August 18th, 2012, 3:57 a.m., Jason A. Donenfeld wrote:









 

  


Review request for Plasma, Jason A. Donenfeld and Aaron J. Seigo.


By Jason A. Donenfeld.




Updated Aug. 18, 2012, 3:57 a.m.








Descripti=
on 

 

 

  
   
Jason's dictionary runner plugin for krunner.

  		
 






Testing <=
/h1>


 

  
   
Works well under various different loads.

  		
 








Diffs=
 


    

 
  • runners/CMakeLists.txt (f487563c58b5bacc5e=
49b9e1a0f6e306956bbf7f)
    • 

 
  • runners/dictionary/CMakeLists.txt (PRE-CRE=
ATION)
    • 

 
  • runners/dictionary/Messages.sh (PRE-CREATI=
ON)
    • 

 
  • runners/dictionary/dictionarymatchengine.h (PRE-CREATION)
    • 

 
  • runners/dictionary/dictionarymatchengine.cpp (PRE-CREATION)
    • 

 
  • runners/dictionary/dictionaryrunner.h (PRE=
-CREATION)
    • 

 
  • runners/dictionary/dictionaryrunner.cpp (P=
RE-CREATION)
    • 

 
  • runners/dictionary/dictionaryrunner_config.h (PRE-CREATION)
    • 

 
  • runners/dictionary/dictionaryrunner_config.cpp (PRE-CREATION)
    • 

 
  • runners/dictionary/plasma-runner-dictionary.desktop (PRE-CREATION)
    • 

 
  • runners/dictionary/plasma-runner-dictionary_config.desktop (PRE-CREATION)
    • 





View Diff





Screensho=
ts 





 =3D"The





  		
 











  

 



--===============3813008473124798931==--

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

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

--===============4996122883384326407==--