Opened 12 years ago

Closed 8 years ago

#1178 closed enhancement (patch_rejected)

Protocol autoswitching should print out a notification in the conversation window

Reported by: cdmarcus Owned by: deryni
Milestone: Patches Needing Improvement Component: pidgin (gtk)
Version: 2.0 Keywords:
Cc: tdfs

Description

It would be nice if the user was notified when their buddy switched between protocols/user accounts under a metacontact. For example, if I'm chatting with a friend for whom I have more than one buddy under a metacontact, I would be notified in the conversation window (just as with the Buddy State Notification plugin) with a message like "Foo Bar has switched from foobar54 (AIM) to foo.bar@… (XMPP)" or even Foo Bar has switched from AIM to XMPP.

Attachments (6)

patch_for_pidgin_gtkconv.h.txt (472 bytes) - added by tdfs 11 years ago.
Patch for pidgin/gtkconv.h
patch_for_pidgin_gtkconv.c.txt (2.3 KB) - added by tdfs 11 years ago.
Patch for pidgin/gtkconv.c
patch_for_pidgin_gtkconv.c-v2.txt (2.4 KB) - added by tdfs 11 years ago.
Patch for pidgin/gtkconv.c version 2 with changes suggested by deryni.
patch_for_pidgin_gtkconv.c-v3.txt (2.4 KB) - added by tdfs 11 years ago.
Patch for pidgin/gtkconv.c version 3.
patch_for_pidgin_gtkconv.c-v4.txt (1.8 KB) - added by tdfs 11 years ago.
Patch for pidgin/gtkconv.c version 4. Patch integrated in pidgin_conv_switch_active_conversation.
patch_for_pidgin_gtkconv.c-v5.txt (777 bytes) - added by tdfs 11 years ago.
Patch for pidgin/gtkconv.c version 5. No need to patch pidgin/gtkconv.h any more.

Download all attachments as: .zip

Change History (37)

comment:1 Changed 12 years ago by deryni

Conversation windows don't currently auto-switch on buddy status changes. They only switch if the buddy changes which account they are sending you messages from, when you select a different buddy from the Send To menu, or if you double-click the contact in the buddy list again. That being said it has recently come up that auto-switching might be a useful feature and if it were added it would certainly print a message (it might even print a message for the current times it can switch if that is decided to be useful).

comment:2 Changed 12 years ago by cdmarcus

deryni, did it recently come up in #pidgin? Because I'm cables on freenode. I submitted three tickets with my requests, which pretty much cover what I was discussing in #pidgin. Should I have consolidated them into one?

comment:3 Changed 12 years ago by deryni

No, I was not discussing your recent conversation in pidgin. I was not aware of that until just a moment ago (it came up a different time), but I still don't believe that pidgin auto-switches the conversation window other than the times I listed, are you saying that it currently does? No, the three separate requests was the right way to handle this but they all assume a behaviour that I am not aware that pidgin has at the moment.

comment:4 Changed 12 years ago by cdmarcus

deryni, one of my requests was that Pidgin autoswitch on status change. What I'm saying here is that it should notify when it autochanges for any reason. I tried to describe the current behavior of changing when the contact messages you on a different account, but i think you misunderstood.

comment:5 Changed 12 years ago by rlaager

  • pending changed from 0 to 1

Why would such a message be necessary?

comment:6 Changed 12 years ago by cdmarcus

  • pending changed from 1 to 0

I'm the type of person who likes to control my computer, rather than vice-versa, so I find it sort of disconcerting when software does things without telling me. Also, some protocols work better than others for certain things, like file transfer, so when my friend switches from AIM to Jabber, I'm often surprised when file transfers don't work.

comment:7 Changed 12 years ago by getisboy

I agree. Pidgin often switches to AIM without telling me. Then when i look for the chat in Google Talk history, it's not there! Pidgin should alert you when your buddy switches accounts.

comment:8 Changed 12 years ago by rlaager

cdmarcus: You should file a separate ticket asking for intelligent behavior there. Instead of printing a notice so you don't get confused, we should simply make file transfer work better under such scenarios. For example, we could use file transfer to the first protocol that supports it.

getisboy: Why is your contact switching from Google Talk to AIM in those cases?

comment:9 Changed 12 years ago by cdmarcus

It's not just that, there are a lot of features that are unique to different protocols, and the autoswitching behavior can be disconcerting. Since there's no preference that sets it on or off, and no notification that it's happening, I've noticed that a lot of people who I set up with Pidgin don't realize what's going on when they've accidentally switched protocols. Also, perhaps getisboy's friend has a work Jabber account, and when he switches to his laptop on lunch break, he switches to AIM. There are a lot of reasons people might switch protocols, and we have to accept that their behavior isn't the problem here. Getisboy's friend may have never used Pidgin, and doesn't know that it won't be obvious for Getisboy that he's switched accounts. This functionality doesn't have to be added to the core client, but perhaps it should be added as an option in the Buddy State Notification plugin.

comment:10 Changed 12 years ago by lschiere

  • Milestone set to 2.0.3

comment:11 Changed 12 years ago by lschiere

  • Owner set to ecoffey

comment:12 Changed 12 years ago by seanegan

  • Milestone changed from 2.0.3 to 2.1.0

Milestone 2.0.3 deleted

comment:13 Changed 12 years ago by ecoffey

Not sure why I got assigned this ticket. But if it wasn't actually a mistake I can attempt to try and figure it out :-)

comment:14 Changed 12 years ago by rlaager

  • Owner ecoffey deleted

comment:15 Changed 12 years ago by seanegan

  • Milestone changed from 2.1.0 to 2.1.1

This will require a new string, so retargetting.

comment:16 Changed 12 years ago by lschiere

  • Milestone changed from 2.2.2 to Patches welcome

comment:17 Changed 11 years ago by tdfs

Hi guys!

I've written a patch for this and I hope you'll like it. This is how it works: before printing an IM to window it calls a function to check if protocol has changed and if yes, it notify the users. To detect if protocol has changed, before printing an IM the protocol is stored in PidginConversation?->account_old (field added in gtkconv.h for the patch). I used PidginConversation? instead of PurpleConversation? because there can be more PurpleConversations? in a windows/tab and PidginConversation? let me select last active PurpleConversation?. To prevent fake protocol changes that would come if using more tabs in the same windows but with different protocols, the notify message is printed only if conversation is active.

diff -u output of gtkconv.h and gtkconv.c will follow. --

TheDarkFreeSoul?

Changed 11 years ago by tdfs

Patch for pidgin/gtkconv.h

Changed 11 years ago by tdfs

Patch for pidgin/gtkconv.c

comment:18 Changed 11 years ago by tdfs

As TipsForPatchSubmissions says... I forgot to say who I am. I'm Alberto Maria 'TheDarkFreeSoul?' Scattolo and my mail is alberto.m.scattolo at gmail dot com. If I'm forgetting anything else, please tell me. -- TheDarkFreeSoul?

comment:19 follow-up: Changed 11 years ago by deryni

Would this feature not be more simply added by making pidgin_conv_switch_active_conversation print out the switch message when it switches conversations?

comment:20 in reply to: ↑ 19 Changed 11 years ago by tdfs

Replying to deryni:

Would this feature not be more simply added by making pidgin_conv_switch_active_conversation print out the switch message when it switches conversations?

Uhmmm... yes it can be an idea but this will not simplify the thing. At the moment I can't test the new patch but I submit it anyway, it should just work. I'll call it patch_for_pidgin_gtkconv.c-v2.txt so we can keep both. -- TheDarkFreeSoul?

comment:21 Changed 11 years ago by rlaager

You haven't really implemented what deryni suggested. You just moved the function call. If you actually move the code, you should be able to simplify it significantly because 1) you don't need to do all the conv/gtkconv variable setup and 2) you should be able to assume that a change is happening rather than have a big if statement checking for it.

comment:22 Changed 11 years ago by tdfs

Ok rlaager I didn't understand it :). I'll write the patch but I can't test it before this evening so I'll post it in 8 or 10 hours, when I go back home. Thanks for helping!

comment:23 Changed 11 years ago by deryni

  • Milestone changed from Patches welcome to Patches Needing Improvement

comment:24 Changed 11 years ago by tdfs

Here comes the patch version 2 with modifications suggested by deryni. I overwrite the existing -v2 patch posted before since, as rlaager said, was not really usefull. I hope you'll like it.

Changed 11 years ago by tdfs

Patch for pidgin/gtkconv.c version 2 with changes suggested by deryni.

comment:25 Changed 11 years ago by rlaager

This -v2 patch doesn't seem any different. Did you perhaps post the old one again? If not, maybe Trac or my browser is screwing up. Could you try reposting it again, as a -v3 or something, so the name is different?

comment:26 Changed 11 years ago by tdfs

Here you are :)

Changed 11 years ago by tdfs

Patch for pidgin/gtkconv.c version 3.

Changed 11 years ago by tdfs

Patch for pidgin/gtkconv.c version 4. Patch integrated in pidgin_conv_switch_active_conversation.

comment:27 Changed 11 years ago by rlaager

  1. The bottom three hunks of the patch are useless changes. You should undo those so they don't show up in the diff.
  1. You're using different indentation. Look at your patch here in the Trac viewer and it should be obvious. Use tabs, not spaces.
  1. The check to see if the protocol changed is not necessary. If you're reaching this code, the account must have changed. (See the "if (old_conv == conv) return;" code on lines 2231-2232.)
  1. Related to point 3, I see no reason to special case changing protocols when you can just print a message on account change. All you really need is:
    message = g_strdup_printf("This conversation has been switched to %s.",
                              purple_account_get_username(conv->account));
    purple_conversation_write(...);
    g_free(message);
    

comment:28 Changed 11 years ago by tdfs

Ok rlaager. Sorry but it's first time I write a patch for pidgin and every project is different from the other. I made all changes you suggested and it works. When I wrote the patch for the first time I didn't notice function pidgin_conv_switch_active_conversation so I had to implement much more things. Now I think it's ok, and at this point, there's no need to patch gtkconv.h. I attach patch_for_pidgin_gtkconv.c-v5.txt. If there are still problems I'm here.

Changed 11 years ago by tdfs

Patch for pidgin/gtkconv.c version 5. No need to patch pidgin/gtkconv.h any more.

comment:29 Changed 11 years ago by tdfs

Hey guys can we close this ticket? Would be nice to see this in 2.5.2 :D

comment:30 Changed 11 years ago by deryni

  • Owner set to deryni

comment:31 Changed 8 years ago by rekkanoryo

  • Resolution set to patch_rejected
  • Status changed from new to closed

Resolving this patch as "patch_rejected" because the issues mentioned here over a year ago have not been addressed. Anyone who wants to pick this patch up and improve it is more than welcome to do so.

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!