Opened 11 years ago

Last modified 4 years ago

#780 new patch

Sound event for new email

Reported by: chrisjames_71 Owned by:
Milestone: Patches Needing Review Component: pidgin (gtk)
Version: 2.0 Keywords:
Cc: djhighmass

Description

Any chance of getting a plug-in for a sound notice for emails? Or even make the icon flash when you have new email? You have to show the buddy list in order to see if you've received mail. Ot even get an envelope to show. Kinda like the one used for new IMs.

Attachments (4)

pidgin-mail-notify.patch (3.5 KB) - added by bl4 10 years ago.
A simple patch for pidgin 2.3.1 source code that makes pidgin play sound and change tray icon when it receives new mail
pidgin-mail-notify-event.patch (6.5 KB) - added by bl4 10 years ago.
A simple patch for pidgin 2.3.1 source code that makes pidgin play sound and change tray icon when it receives new mail. It adds a separate sound event.
pidgin-mail-notify-event.unified.patch (5.4 KB) - added by bl4 10 years ago.
The same patch made with diff -u
pidgin-mail-notify-event.updated.patch (5.9 KB) - added by bl4 10 years ago.
updated patch

Download all attachments as: .zip

Change History (40)

comment:1 Changed 11 years ago by lschiere

  • Summary changed from Sounds to Sound event for new email

comment:2 Changed 11 years ago by sadrul

I think any event that sets the URGENT flag for the buddylist window should cause the docklet to blink when the buddylist is not being displayed. The events that currently set URGENT for blist window are: (1) new mail, and (2) new auth request.

I don't use sound. But I think it kind of makes sense to have sound events for new mails.

comment:3 Changed 11 years ago by gryn

I'm a keyboard junkie: Right now, I will often miss new emails if i rely on pidgin. I can certainly download my gmail mail notification tool, but all I want is the icon to change, and if you want to add a sound that's groovy too.

Right now, the buddy list is almost never open, because I'll have a chat window open instead. Then I'll hit control-m to start chatting to a new guy -- using the mouse to open the buddy list isn't my style. (Besides, going to hit a 16x16 icon to do a task isn't great UI [it's against fitt's law]).

BTW, gluefications (sp?) will notify me if I have a new email, but i get so many of them, that one doesn't stand out.

P.S. When I check email independently of pidgin (and thus the email is no longer new) pidgin insists that there are still new emails.

comment:4 Changed 10 years ago by lazybones

I would also really like to have a sound for new mail.

I also believe that the tray icon should show some form of notice of new email.

I also find it anoying that Pidgin does not clear the email notice after having read all the messages without clicking on the notice.

comment:5 Changed 10 years ago by ebx

This patch looks great. Will it be applied in future as a default option?

In the meantime, I use pidgin on windows as well as bsd. How do I get at the source c file to apply this patch to my win release?

Thanks

comment:6 Changed 10 years ago by ebx

I found instructions here:

http://developer.pidgin.im/wiki/BuildingWinPidgin

trying it now...

comment:7 Changed 10 years ago by ebx

I've built a win release using the instructions above with the patch. it does change the tray icon but doesn't play a sound.

comment:8 Changed 10 years ago by ebx

I can't explain it, but it works now.

I changed the patch for gtknotify.c from

purple_sound_play_event(PURPLE_SOUND_FIRST_RECEIVE, account); play sound

to

purple_sound_play_event(PURPLE_SOUND_RECEIVE, NULL); play sound

(meaning message is received event and NULL meaning for all accounts)

However, when I first made this change it did nothing. It was only after I put the machine into hibernation and then brought it back up that it began to work. I had to set message received as being checked. The problem with this is that now I have to have that item checked at all times.

It would be best to add a custom event that is only for mail is received, as I don't want to hear a noise every time I get an im or even when an im begins conversation, just when mail arrives.

The compromise for me is to use the "someone says your name in chat" event as it is not likely to occur. That may not be the case for others. This is the change:

purple_sound_play_event(PURPLE_SOUND_CHAT_NICK, NULL); play sound

Changed 10 years ago by bl4

A simple patch for pidgin 2.3.1 source code that makes pidgin play sound and change tray icon when it receives new mail

comment:9 Changed 10 years ago by bl4

Thanks for your help. I've updated the patch to use PURPLE_SOUND_RECEIVE event. I wanted pidgin to play sound as if first message has come when new mail is received but now I see that it's disabled by default.

Both account and NULL work for me as the second parameter. I don't really understand what purple accounts are but I think NULL should work in all cases so I've changed it too.

I've also fixed a problem with tray icon. It changed when there was new mail, but it was restored when status changed. If you had auto-away enabled and you received mail while you were away, and then you came back and your status changed, the icon would show your status, not new mail.

For those who don't know how to apply the patch (on unix-like systems):

cd path_to_pidgin_source
patch -p0 < path_to_patch

After that you need to recompile and reinstall pidgin.

comment:10 Changed 10 years ago by ebx

Awesome, nice changes. If I had time and new how I'd love to change the UI and add a special sound event for new mail, so it could be independent from the others. Maybe I'll try and learn enough to do that and make a patch :-) but I think there are people who could do it better and faster.

Thanks for the great patch!

Changed 10 years ago by bl4

A simple patch for pidgin 2.3.1 source code that makes pidgin play sound and change tray icon when it receives new mail. It adds a separate sound event.

comment:11 Changed 10 years ago by bl4

Now I've done this too. This new patch adds a separate sound event for new mail. It can be switched on or off in sound options. It wasn't difficult to make because the sound options dialog is generated dynamically, all I had to do was find the list of events and make some changes to the code.

comment:12 Changed 10 years ago by hannibal90210

This patch really needs to be integrated into the next version of Pidgin. Compiling takes too long on an old PIII. Anyways, many thanks bl4 for this very handy patch, it works a treat! Much appreciated.

comment:13 Changed 10 years ago by seanegan

  • Type changed from enhancement to patch

comment:14 Changed 10 years ago by rekkanoryo

This patch can't go in for Pidgin 2.4.0, as we are string frozen pending release--no new strings and no string changes allowed. It could go in for a future release, though.

bl4: could you please submit the patch as a unified diff (-u argument to diff)? The patch is difficult to follow in its current form.

Changed 10 years ago by bl4

The same patch made with diff -u

comment:15 Changed 10 years ago by adot

I would really like to see this integrated into the next Windows release, since I use Pidgin not just for the convenience of using 6 different accounts at once, but also because it lets me avoid having to login and check my 6 different email accounts. I am a big user of audio notifications and would love to see this patch in it's "separate sound event" form integrated into the next release of winpidgin, since I am a student who does not have time to learn how to compile my own builds right now, much as I'd like to. It would be much appreciated.

comment:16 follow-up: Changed 10 years ago by ebx

I agree. Even though I know how to compile and apply diffs and all that, it's a bit of a pain. I rarely update since the win releases don't usually have many changes, but honestly it has more to do with not wanting to compile the sucker with the sound events. It seems like these changes could be applied to the source by the maintainers very easily.

Do they read this forum?

comment:17 in reply to: ↑ 16 Changed 10 years ago by rekkanoryo

Replying to ebx:

Do they read this forum?

We do. There are some problems. The patch isn't a shining example of acceptibility. It needs work. That work takes time and effort that thus far no one has expended. Finding the motivation to do such work is often difficult.

I've been meaning to take this patch and rewrite it, but I haven't gotten around to it yet.

comment:18 follow-up: Changed 10 years ago by bl4

True, it needs work, more patches like this would surely make a mess in the code. Why didn't you write what exactly is unacceptable and why? It's been 5 months since I uploaded this patch. I understand you don't have time to rewrite it but I'd like to help too. If you give some hints what you don't like about this patch, I will do my best to improve it until it's good enough to be accepted.

comment:19 Changed 10 years ago by rekkanoryo

Ok, a couple notes then:

  • // style comments are unacceptable. We try to remain C89 (aka ANSI C) compliant. The only type of valid comment in C89 is /* ... */.
  • You added a public function is_new_mail that is in no way namespaced. It should fit into the function namespace presented in the file you added it to.
  • Ideally I'd like to see a signal emitted for new mail and have modifications to gtksound.c to attach to the signal instead. If you observe the rest of gtksound.c, you'll see that a number of sound events are triggered by libpurple signal emissions.

There are other issues that I had with this patch, but I don't recall them at the moment.

comment:20 Changed 10 years ago by ebx

It kind of sounds like the pidgin development team doesn't release anything in the way of standards for contributors. Is that a fair observation?

I know there is a Coding Standards page, but it's pretty light and doesn't cover anything in the way of the things mentioned above.

I guess my point is it's a bit of a downer for people trying to make pidgin better to have nothing much to go on until after it's been reviewed and (imo) somewhat poo-poo'ed.

This is a great addition to pidgin, should be boiler plate by now and rather than being met with vague criticisms, should be encouraged and helped into the codebase.

My two cents. We can always patch and build independently. Yet, how many people who use other multi-protocol clients are used to sound notifications for mail and don't find it with pidgin?

And for the record, having looked over a lot of the pidgin code, I can't say I see what I would call true standards being adhered to throughout the build, so I'm kind of mystified by all this.

comment:21 Changed 10 years ago by deryni

Feel free to add the above comments to the StyleGuide page, it is a wiki after all. The existing code is a decent guide to the pidgin standards (it varies in places for certain things unfortunately, but generally not too much), generally making the added code match as closely as possible the code it is being added to will minimize the problems.

Last edited 4 years ago by MarkDoliner (previous) (diff)

comment:22 Changed 10 years ago by rlaager

  • Milestone set to Patches Needing Improvement

comment:23 in reply to: ↑ 18 Changed 10 years ago by rlaager

Replying to bl4:

Why didn't you write what exactly is unacceptable and why? It's been 5 months since I uploaded this patch.

I've created a "Patches Needing Improvement" milestone and a couple of wiki pages (PatchesNeedingImprovement and PatchesNeedingReview) to hopefully help us stay on top of this better. Of course, we'll still need eyes on the patches, but sorting out which ones are blocked on us and which are blocked on the submitters will help a lot, I hope.

Changed 10 years ago by bl4

updated patch

comment:24 Changed 10 years ago by bl4

Well, I'm not going to block it any longer. Here's the updated patch, I've changed everything as rekkanoryo suggested:

  • Sound for new email is played similar to other sound events, in a callback function which is associated with the libpurple signal for new email.
  • Public function namespacing: renamed is_new_mail to pidgin_notify_is_new_mail
  • Comments

Other changes:

  • Eliminated pidgin_docklet_update_icon_mail function. Instead, pidgin_docklet_update_icon recognizes if there's new mail.
  • Moved pidgin_notify_is_new_mail declaration between #ifndef ... #endif in the header file, so it can be declared only once.


I haven't had much time for testing but these aren't big modifications and everything should work like before. I hope this code matches pidgin code better. Please let me know if there are any other issues.

I've just noticed that when I click on the attachment name, a preview of the patch is displayed, but it's incomplete. Only modifications for 3 files are shown, while this patch modifies 6 files.

comment:25 Changed 10 years ago by rlaager

  • Milestone Patches Needing Improvement deleted

comment:26 Changed 10 years ago by deryni

The fact that the trac diff view occasionally eats parts of attached patches is something we know about though I don't believe we know why it happens. Thanks for pointing out that it hit your patch since I know I've more than once been bitten commenting on patches that appear to be incomplete because I don't view the raw diff.

comment:27 Changed 9 years ago by lschiere

has someone had a chance to look at this patch since deryni found that trac hadn't displayed it properly?

comment:28 Changed 8 years ago by rekkanoryo

  • Milestone set to Patches Needing Review

comment:29 Changed 8 years ago by adot

really looking forward to this being implemented... ...sorry, I understand all the devs here are volunteers, but according to the posts here, this has been ready for implementation for over a year. I would compile my own build but I hardly know the first thing about programming.

Having this sound event would increase my productivity; I work from home and monitor my business email account using Pidgin and I have to turn on my LCD every hour or so to see whether a notification has popped up. Having the new mail sound event would allow me to continue working uninterrupted and simply be notified with the sound when I have a new email; it seems to be the point of a "notification" in the first place---to not have to constantly be looking.

comment:30 Changed 8 years ago by sadrul

I am OK with the sound-event bit (I am not sure the patch for finch is going to work, btw, but don't worry about that).

But I think the change to make the docklet blink is not very ... clean.

comment:31 Changed 8 years ago by bl4

You may as well close this ticket because with this kind of attitude it's unlikely that anything will happen in the next 4 years.

comment:32 Changed 8 years ago by Hunkah

Is it possible to convert these patches into plugins? I know nothing about coding, but with my small-minded thinking, a plugin wouldn't depend on any one person to implement. That way the devs can do their thing, and we can have functionality... No?

Just a suggestion... let me know if I am totally stupid, and I will bury my head again.

comment:33 Changed 7 years ago by jsczar

@sadrul Can we at least see the sound-event portion implemented for the next release? This was proposed 4 years ago. Perhaps the docklet blink can be split off until it is "cleaned up" and then included later OR maybe it could be a separate config option (off by default) like: [ ] Blink icon (experimental)

comment:34 Changed 6 years ago by heldersepu

I'm very happy with Pidgin overall, but today I had to switch to Google Talk because of this 5 year old missing feature...

comment:35 Changed 6 years ago by heldersepu

In an interesting note, GMail Notifier uses the default sound in the control panel: http://email.about.com/od/gmailtips/qt/How_to_Change_the_New_Mail_Sound_for_Gmail.htm

comment:36 Changed 5 years ago by Jorge Villaseñor <salinasv@…>

(In [c51c70f5545a]):
Add email notification in the docklet area.

Patch from Alexei Potashnik updated and adapted by me plus some bug fixes (cleaning the dock when closing the headline in the blist) Fixes #3571 Refs #780

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!