DevHeads.net

Review Request: Enable kdialog to set an icon for passivepopup as well

Review request for KDE Base Apps.

Summary
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 development 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: <a href="http://git.reviewboard.kde.org/r/101282/diff" title="http://git.reviewboard.kde.org/r/101282/diff">http://git.reviewboard.kde.org/r/101282/diff</a>

Testing

Thanks,

Kai Uwe

Comments

Re: Review Request: Enable kdialog to set an icon for passivepop

By Kai Uwe Broulik at 05/05/2011 - 11:57

(Updated May 5, 2011, 3:57 p.m.)

Review request for KDE Base Apps.

Changes
The --icon parameter did not work as expected, so I added a new one --passiveicon which purpose is only to set the icon for --passivepopup.
Compiled fine, tested it and icon is changeable for passivepopup.
Usage: kdialog --passivepopup "your message" --passiveicon "someicon"

Summary
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 development 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 (updated)
kdialog/kdialog.cpp b80ad9a

Diff: <a href="http://git.reviewboard.kde.org/r/101282/diff" title="http://git.reviewboard.kde.org/r/101282/diff">http://git.reviewboard.kde.org/r/101282/diff</a>

Testing

Thanks,

Kai Uwe

Re: Review Request: Enable kdialog to set an icon for passivepop

By Kai Uwe Broulik at 05/06/2011 - 08:15

(Updated May 6, 2011, 12:15 p.m.)

Review request for KDE Base Apps.

Changes
Changed icon argument parsing to const ref.
Added icon support for fallback KPassivePopup using KIconLauncher.
Could not test fallback, though, since nothing happens when I close plasma-desktop. It used to display the KPassivePopup then. Neither with 4.6, nor trunk, nor my patch it does anything.
Plasma passive notification works fine.

Summary
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 development 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 (updated)
kdialog/kdialog.cpp b80ad9a

Diff: <a href="http://git.reviewboard.kde.org/r/101282/diff" title="http://git.reviewboard.kde.org/r/101282/diff">http://git.reviewboard.kde.org/r/101282/diff</a>

Testing

Thanks,

Kai Uwe