[KDE/Mac] Review Request 128043: Allow to play the default system sound (beep) instead of a specific sound file (WIP)

[René J.V. Bertin]

> On May 30, 2016, 7:38 p.m., Martin Klapetek wrote:
> > src/notifybyaudio.cpp, line 61
> > <https://git.reviewboard.kde.org/r/128043/diff/1/?file=467231#file467231line61>
> >
> >     But this is not correct, if you disable the sound in the config, the soundFilename will be empty and this will cause ::beep() to be executed. But why if I choose no sound?
> 
> René J.V. Bertin wrote:
>     Hmmm, I didn't think of it that way. The warning message suggests that this is a case of an incomplete notification definition. Why would you choose to have an audio notification that doesn't make a sound? Shouldn't that be handled by another special case, something like `Sound=silent` or `Sound=none`?
>     
>     Or is the warning text incorrect?

Bump, any suggestions how to handle this?

Also, what exactly do you mean with "if you disabled the sound in the config"? Do you mean untick the  "Play a sound" option? If so, why would a notification that is not supposed to make a sound call `NotifyByAudio::notify()`?


- René J.V.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128043/#review96047
-----------------------------------------------------------


On June 9, 2016, 7:52 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128043/
> -----------------------------------------------------------
> 
> (Updated June 9, 2016, 7:52 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: knotifications
> 
> 
> Description
> -------
> 
> This is a follow-up to discussion on the plasma-devel ML. When not running under a Plasma session it would be preferable if generic alert sounds are (can be) delivered using the default system sound. The easiest way to do that is to call `QApplication::beep()` ; at least on OS X this will play the user's selected alert sound.
> 
> I have implemented this for now by assuming the existence of a virtual (relative) `beep` sound file that always plays the default sound. 
> 
> In addition, this patch provides better warning feedback if a soundfile isn't found (making it easier to figure out which alert is misconfigured), and will play the sysbeep in that case (so that the alert doesn't go lost if it's sound-only).
> Finally, no Phonon::AudioOutput object is created until that is actually required.
> 
> 
> Diffs
> -----
> 
>   src/notifybyaudio.cpp 903e996 
> 
> Diff: https://git.reviewboard.kde.org/r/128043/diff/
> 
> 
> Testing
> -------
> 
> Works as expected on OS X 10.9.5
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20160702/22d090da/attachment.html>