From kde-core-devel Tue May 10 09:36:50 2011 From: "Kai Uwe Broulik" Date: Tue, 10 May 2011 09:36:50 +0000 To: kde-core-devel Subject: Re: Review Request: Enable kdialog to set an icon for passivepopup as Message-Id: <20110510093650.19193.37935 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kde-core-devel&m=130509683001642 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============4632190832840673379==" --===============4632190832840673379== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On May 9, 2011, 7:37 p.m., David Palacio wrote: > > kdialog/kdialog.cpp, line 366 > > > > > > --icon is a KDE option. > > Maybe that's why the first diff did not work? I=E2=80=99ll try again using --icon KDE option > On May 9, 2011, 7:37 p.m., David Palacio wrote: > > kdialog/kdialog.cpp, line 101 > > > > > > s/duration/timeout > > There's no need to change this name here and you change it backward= s some lines below. KDialog's usage text names this option "timeout" as wel= l. That was some experiment and I forgot to completely revert it. Will change = it ;) - Kai Uwe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101282/#review3228 ----------------------------------------------------------- On May 6, 2011, 2:52 p.m., Kai Uwe Broulik wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101282/ > ----------------------------------------------------------- > = > (Updated May 6, 2011, 2:52 p.m.) > = > = > Review request for KDE Base Apps. > = > = > Summary > ------- > = > Since I use kdialog a lot, I wanted it to be able to set an icon for pass= ive Plasma notifications as well. > I saw that the icon was hardcoded to dialog-information and tried to enab= le it to parse the --icon command line argument and pass it to the dbus cal= l that triggers the notification. > I could not test the patch however, since I do not (yet) have a full deve= lopment environment or its dependencies, and I do not even know if this is = the right approach, but I hope you can have a look at it. > = > = > Diffs > ----- > = > kdialog/kdialog.cpp b80ad9a = > = > Diff: http://git.reviewboard.kde.org/r/101282/diff > = > = > Testing > ------- > = > = > Thanks, > = > Kai Uwe > = > --===============4632190832840673379== 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/101282/

On May 9th, 2011, 7:37 p.m., David Palacio<= /b> wrote:

= = =
kdialog/kdialog.cpp (Diff revision 1) 
static int directCommand(KCmdLineArgs *args)
 

    366
    
	KCmdLineArgs *qtargs =3D KCmdLineArgs::parsedArgs(&q=
uot;qt"); // --icon=
 is a qt option

  


 




  
--icon is=
 a KDE option.
Maybe that's why the first diff did not work?

 









I=E2=80=99ll try again using --icon KDE option






 
On May 9th, 2011, 7:37 p.m., David Palacio<=
/b> wrote:

 

  =








 =

 =





 

 
  

   
    kdialog/kdialog.cpp
    

     (Diff revision 5)

    
   
  

 

 
  


   
bool sendVisualNotification(QString text, QString title, int timeou=
t)

   


  

 

  

    99
    
bool sendVisualNotification(QString text, QString title, int timeout)

    101
    
bool sendVisualNotification(QString text, const QString =
&title,=
 const QString &=
icon, int duration)

  


 




  
s/duratio=
n/timeout
There's no need to change this name here and you change it backwards so=
me lines below. KDialog's usage text names this option "timeout&qu=
ot; as well.

 









That was some experiment and I forgot to completely revert it. Will =
change it ;)








- Kai Uwe






On May 6th, 2011, 2:52 p.m., Kai Uwe Broulik wrote:









 

  


Review request for KDE Base Apps.


By Kai Uwe Broulik.




Updated May 6, 2011, 2:52 p.m.






Descripti=
on 



 

  
   
Since I use kdialog a lot, I wanted it to be able to set an =
icon for passive Plasma notifications as well.
I saw that the icon was hardcoded to dialog-information and tried to enable=
 it to parse the --icon command line argument and pass it to the dbus call =
that triggers the notification.
I could not test the patch however, since I do not (yet) have a full develo=
pment environment or its dependencies, and I do not even know if this is th=
e right approach, but I hope you can have a look at it.

  		
 









Diffs=
 


    

 
  • kdialog/kdialog.cpp (b80ad9a)
    • 





View Diff





  		
 











  

 



--===============4632190832840673379==--