Opened 9 years ago

Closed 7 years ago

Last modified 18 months ago

#4598 closed patch (fixed)

Plus! color tags

Reported by: daniele_athome Owned by: salinasv
Milestone: 2.7.0 Component: pidgin (gtk)
Version: 2.3.1 Keywords: pidgin blist markup
Cc: jgoguen, Universe_JDJ, olivier, darkweasel, MastroPino, frasten, markuznw, salinasv, cristi8

Description

I wrote a patch that registers a new signal in the gtkblist.c file. That signal will be called one moment before g_escape_markup_text will be called. The callback will return an already-escaped version of the name.

I also wrote a test plugin that makes all the buddies red :)

Attachments (8)

plusblist.zip (312.7 KB) - added by daniele_athome 9 years ago.
This zip file contains both the test plugin and a patch for gtkblist.c
pidgin-plus-0.0.2.tar.gz (313.6 KB) - added by daniele_athome 9 years ago.
the test plugin now parses all the plus! tags in buddy list, but doesn't make gradient nicks.
plusblist.patch (1.8 KB) - added by resiak 9 years ago.
patch extracted from plusblist.zip
bug4598-plus_colour-cleaned.patch (2.3 KB) - added by jgoguen 9 years ago.
Attempt at cleaning up the previous patch for this ticket.
bug4598-plus-long_closing_tags.patch (249 bytes) - added by abesto 9 years ago.
Accept /c=#RRGGBB style closing tags.
plusblist_new.patch (1.8 KB) - added by daniele_athome 8 years ago.
Upgraded Pidgin patch to gtkblist.c
pidgin-plus-0.0.2_new.tar.gz (313.0 KB) - added by daniele_athome 8 years ago.
Upgraded plugin for Pidgin - version 1 (without gradient colors)
plusblist_fixed.patch (1.8 KB) - added by daniele_athome 8 years ago.
Fixed a memory leak

Download all attachments as: .zip

Change History (76)

Changed 9 years ago by daniele_athome

This zip file contains both the test plugin and a patch for gtkblist.c

comment:1 Changed 9 years ago by datallah

  • Type changed from enhancement to patch

Changed 9 years ago by daniele_athome

the test plugin now parses all the plus! tags in buddy list, but doesn't make gradient nicks.

Changed 9 years ago by resiak

patch extracted from plusblist.zip

comment:2 Changed 9 years ago by resiak

For future reference, don't put patches into zip archives, just attach them straight to the ticket.

comment:3 Changed 9 years ago by rekkanoryo

There are a number of problems I see with this patch right off the bat:

  • comments are not valid ANSI C.
  • Use of g_debug is strongly discouraged. Use the libpurple-provided debugging API, which is covered quite well in the API documentation and the plugin howto wiki pages.
  • Parameters to functions need spaces between them. For example, foo(parm1, parm2) instead of foo(parm1,parm2). The lack of spaces makes code very unreadable.

comment:4 Changed 9 years ago by kasmol

Great!! Please continue to develop it!!

comment:5 Changed 9 years ago by Twain28

Anything new about this? People on MSN are increasingly using bbcode to set colors, styles and text dimentions....this patch could come in handy.

comment:6 Changed 9 years ago by QuLogic

  • Milestone set to Patches Needing Improvement

comment:7 Changed 9 years ago by jgoguen

CC myself

Changed 9 years ago by jgoguen

Attempt at cleaning up the previous patch for this ticket.

comment:8 Changed 9 years ago by ekerazha

Any chance to see this patch included?

comment:9 Changed 9 years ago by olivier

Great plugin ! The 2.5.0 milestone of pidgin is really fantastic for new MSN features. But I've got many friends who *need* (that's what they say) colored plugins, so they use Windows Live Messenger and Plus. daniele_athome, did you stop working on this ? Maybe I'll try to add gradient nicks support. Would it be possible to have smileys in nicks too ? I'm not familiar with pango.

comment:10 Changed 9 years ago by daniele_athome

This plugin was just an experiment to see if the community was interested to this feature. Personally I don't even like Microsoft MSN protocol and its IM chat system (bad designed protocol, very slow servers). Only some countries and only specific group of people use MSN messenger with colorful nicks (es. very little children or Italians people (sigh)). I'm doing my word hard trying to persuade my friends to switch to the jabber protocol: people let's realize it, MSN is becoming to have its age.

Anyway, if someone is really interested in continuing development (instead of porting friends to jabber :) I will be more than happy to discuss and to explain the plugin code.

comment:11 Changed 9 years ago by olivier

I understand and totally agree, but I've got a different approach. I try to make people switch to another, better, MSN client (instead of Live Messenger) and then convince them to have an XMPP account, that's easy because pidgin supports multiple accounts and protocols. I also have friends who would switch to Linux instead of Windows, but the only show-stopper is "what ? there's no colored nicks in msn ?".

People will always use crappy things. I can't convince all my friends/familiy to switch to another IM protocol, or (for example) to not use this commercial thing called facebook. With this "msn plus nicks" plugin, pidgin will be more attractive to the msn "plus" userbase, and that's a progress.

Do you have a more recent snapshot than the attached 0.0.2 sources, so that the community can continue developing it ?

comment:12 Changed 9 years ago by daniele_athome

No I'm sorry I have not. Should it be compatible with the current version of piding, shouldn't it? (at least the plugin, the patch is too old obviously, but it is pretty simple to reimplement)

comment:13 Changed 9 years ago by olivier

It's ok with the current version (pidgin-plus-0.0.2.tar.gz). I tested it today with pidgin 2.5.0 + bug4598-plus_colour-cleaned.patch

comment:14 Changed 9 years ago by daniele_athome

:-)

Changed 9 years ago by abesto

Accept /c=#RRGGBB style closing tags.

comment:15 follow-ups: Changed 9 years ago by elb

This patch looks like a memory leak waiting to happen -- it strdup()s a string returned from a signal and discards the signal result. I would generally expect the signal handler in a situation like this to have allocated the string it wished to return.

It's also not clear to me why this signal is named "drawing-buddy" and takes a buddy as an argument, but seems to be utilized only to return markup for the buddy's username. A more specific signal name which indicates this rather limited scope would be appropriate here.

It looks to me like the whole purpose of this plugin is to handle MSN's ill-conceived username markup language; this is something which should properly be handled by the MSN protocol plugin. Perhaps a signal should be emitted from the MSN protocol plugin to the effect of msn-buddy-parse-markup, with a default implementation of simply stripping these codes from the nickname.

comment:16 in reply to: ↑ 15 Changed 9 years ago by QuLogic

Replying to elb:

It looks to me like the whole purpose of this plugin is to handle MSN's ill-conceived username markup language; this is something which should properly be handled by the MSN protocol plugin. Perhaps a signal should be emitted from the MSN protocol plugin to the effect of msn-buddy-parse-markup, with a default implementation of simply stripping these codes from the nickname.

This is not technically part of the MSN protocol. It is part of Messenger Plus!, which hacks on various things like coloured nicks, skins, funky logs, and who knows what else.

comment:17 Changed 9 years ago by darkweasel

Personally, I think that you don't need to make them colored, just allowing these [c=15][b]foobc=24 to be cancelled out, so that something like "[b][c=9][a=1]lenii <3acb" just displays as "lenii <3" (that's a real nick of one of my buddies, btw).

comment:18 Changed 8 years ago by MastroPino

I think that this plugin need to be improve, we need some option like a double visualization, I mean plus and anti-plus for example. And I don't understand why this is resting a patch, c'mon every one that use that stupid MSN protocol have for sure a contact in his/her buddy list that use plus tag! Personally every my account use it! Please some one improve this essential patch.

comment:19 Changed 8 years ago by markuznw

Please someone keep up this patch, as of italian user it's pretty impossible to live with a contact list flooded of bbcode tags from plus.

comment:20 Changed 8 years ago by daniele_athome

I decided to regain the development of this plugin again, I'll begin in the next days, as soon as I can. Thank you to everyone for your vote and comments.

comment:21 Changed 8 years ago by Universe_JDJ

That's great news! As I'm primarily a MSN user, my contact list has been growing more and more unreadable as time goes by. Thanks for picking this up again.

comment:22 Changed 8 years ago by daniele_athome

I think you all should switch to jabber :P

comment:23 Changed 8 years ago by Universe_JDJ

Unfortunately, switching average users over to another protocol never works. In my area, everyone uses MSN exclusively, and it's essentially impossible to change that.

comment:24 Changed 8 years ago by MastroPino

At the end I find an italian user that compily pidgin with this patch and he has integrated the plus plugin. please check is site and is repository here: http://polpoinodroidi.com/2009/03/09/il-mio-primo-repository/ or http://polpoinodroidi.com/

Grazie Polpoino!

comment:25 follow-up: Changed 8 years ago by markuznw

Well thanks for the link mastropino but it would be nice to have it included into mainstream and maybe compiled for windows too. I don't actually know any windows multi im client that have color nicks support for msn :P Waiting for updated version

comment:26 in reply to: ↑ 25 Changed 8 years ago by MastroPino

Replying to markuznw:

Well thanks for the link mastropino but it would be nice to have it included into mainstream and maybe compiled for windows too. I don't actually know any windows multi im client that have color nicks support for msn :P Waiting for updated version

This is good if u wanna try or are lazy for compile it I hope that it will be integrated but I loose all hope!

Changed 8 years ago by daniele_athome

Upgraded Pidgin patch to gtkblist.c

Changed 8 years ago by daniele_athome

Upgraded plugin for Pidgin - version 1 (without gradient colors)

comment:27 Changed 8 years ago by daniele_athome

I fixed some little things, and now it works with Pidgin 2.5.5. I didn't try it on other platform, but It should work anyway.

Next step: gradient colors :-)

comment:28 Changed 8 years ago by daniele_athome

FOR PIDGIN ADMINS: I think you should include my patch in trunk, because it may be useful for other purposes, besides Plus! color tags.

comment:29 Changed 8 years ago by deryni

You need to modify the patch to address the issues (the first two at least) brought up by elb.

comment:30 in reply to: ↑ 15 Changed 8 years ago by daniele_athome

Replying to elb:

This patch looks like a memory leak waiting to happen -- it strdup()s a string returned from a signal and discards the signal result. I would generally expect the signal handler in a situation like this to have allocated the string it wished to return.

Already fixed, I'll post the upgraded patch tomorrow morning.

Replying to elb:

It's also not clear to me why this signal is named "drawing-buddy" and takes a buddy as an argument, but seems to be utilized only to return markup for the buddy's username. A more specific signal name which indicates this rather limited scope would be appropriate here.

The name is absolutely not self-explaining, you can change with anything you want :) The handler should a have a buddy because of the information it can retrieve; passing only the buddy alias string could not be enough; for example, the plugin should colorize the buddy only if the user hasn't defined a custom alias.

Replying to elb:

It looks to me like the whole purpose of this plugin is to handle MSN's ill-conceived username markup language; this is something which should properly be handled by the MSN protocol plugin. Perhaps a signal should be emitted from the MSN protocol plugin to the effect of msn-buddy-parse-markup, with a default implementation of simply stripping these codes from the nickname.

In windows, Plus! nicks are managed by a windows live messenger patch (that is messenger plus!), and it is not a protocol extension; all is based on having that patch installed; that patch will render colored nicks correcly.

Changed 8 years ago by daniele_athome

Fixed a memory leak

comment:31 Changed 8 years ago by daniele_athome

I think I accidentally gave a negative vote to the ticket :( admins, can you fix? It should be 5 votes...

comment:32 Changed 8 years ago by markuznw

http://code.google.com/p/pidgin-plus/ keep an eye on this, maybe someone is starting a similar project :p

comment:33 follow-up: Changed 8 years ago by echelon89

I think that bb code sould be removed in the dialog that appears when you go over a contact with the mouse

comment:34 Changed 8 years ago by darkrain42

Masca just pointed out that this patch won't affect the drawing of the buddy name in the conversation window, which is problematic.

comment:35 in reply to: ↑ 33 Changed 8 years ago by MastroPino

Replying to echelon89:

I think that bb code sould be removed in the dialog that appears when you go over a contact with the mouse

Yes and in the notify too!

comment:36 follow-up: Changed 8 years ago by markuznw

Hey, any updates about gradients and/or issues pointed out by darkrain? :)

comment:37 in reply to: ↑ 36 ; follow-up: Changed 8 years ago by daniele_athome

Replying to markuznw:

Hey, any updates about gradients and/or issues pointed out by darkrain? :)

Hi, I have very little time for developing this plugin, I will let you know about my progress as soon as I have something.

comment:38 in reply to: ↑ 37 Changed 8 years ago by MastroPino

Replying to daniele_athome:

Replying to markuznw:

Hey, any updates about gradients and/or issues pointed out by darkrain? :)

Hi, I have very little time for developing this plugin, I will let you know about my progress as soon as I have something.

Daniel thanks for ur work =)

comment:39 Changed 8 years ago by darkrain42

Ticket #10102 has been marked as a duplicate of this ticket.

comment:40 Changed 8 years ago by daniele_athome

Hi, despite of my efforts finding the time for this plugin is very hard. I have very limited time, so I must abandon the development. However, another developer took the code and continued development, implementing gradient nicks too:
http://polpoinodroidi.com/2009/10/01/pidgin-plus-con-gradienti/

Page is in Italian, I'm posting some urls for your convenience:

PPA repository: https://launchpad.net/~frasten/+archive/ppa/+packages
GIT repository: http://github.com/frasten/pidgin-plus

comment:41 Changed 7 years ago by MastroPino

Please tha patch it's really awesome and needed from every user on MSN. Don't let us pray you for this pls xD

comment:42 Changed 7 years ago by salinasv

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

Since we don't want that kind of behavior enabled by default nor maintained by pidgin developers and there is a [looks like working] 3° party plugin being actively developed, I'm going to close this ticket as fixed.

comment:43 Changed 7 years ago by frasten

@salinasv: no wait. I'm the current developer of the 3rd party plugin. But that plugin won't work until you merge the attached patch.

I've been waiting for its inclusion for many months, but noone did that. Please test it and include it into the main trunk.

All the issues reported in this ticket are fixed in the last version of the patch. Please read comment #30 above, where it's explained.

comment:44 Changed 7 years ago by salinasv

  • Resolution fixed deleted
  • Status changed from closed to new

I will take a closer look at the patch sometime this week. I'm reopening it.

comment:45 Changed 7 years ago by salinasv

  • Owner set to salinasv

comment:46 Changed 7 years ago by darkrain42

My comment from 7 months ago (attributed to salinasv a.k.a. Masca) is IMHO still pertinent: http://developer.pidgin.im/ticket/4598#comment:34

comment:47 Changed 7 years ago by salinasv

I also reread elb's comment about this patch being msn-specific and I kinda agree with him about it being msn-specific and must be added to msn-prpl but I'm not sure if some other prpls or plugins can benefit from this patch.

comment:48 Changed 7 years ago by frasten

This patch could allow a plugin to process the buddy's name in the buddy list. In this case it is used to translate msn plus tags, but it could be easily adapted for other purposes.

comment:49 Changed 7 years ago by frasten

About comment 34, are any other signals to process a buddy name everywhere? It would sure be a better choice.

comment:50 Changed 7 years ago by felipec

Does this patch has chances of getting merged? It seems to be a popularly requested feature here and msn-pecan's tracker.

comment:51 Changed 7 years ago by elb

As of plusblist_fixed.patch, it seems to me we're just waiting for 2.7.0.

comment:52 Changed 7 years ago by darkrain42

  • Milestone changed from Patches Needing Improvement to 2.7.0

comment:53 Changed 7 years ago by sadrul

It should be using purple_marshal_POINTER__POINTER

There's no reason to debug-print the name returned from a plugin as a warning. (We probably don't need to print anything in the debug window at all)

Also, ret and the extra strdup is not necessary:

	/* Raise a contact pre-draw signal here.  The callback will return an already  
	 * escaped version of name. 
	 */
	esc = purple_signal_emit_return_1(pidgin_blist_get_handle(), "drawing-buddy", b);
	if (!esc)
		esc = g_markup_escape_text(name, strlen(name));

comment:54 Changed 7 years ago by sadrul

PS: Before someone starts blogging over this, the last post was meant as a note for whoever is going to commit it, not a request for a new patch with the suggested trivial changes.

comment:55 Changed 7 years ago by cristi8

I'm not sure if I got the point, but maybe this patch will also help http://developer.pidgin.im/ticket/3109.

comment:56 follow-up: Changed 7 years ago by rekkanoryo

For the record, I nominate sadrul to commit this patch :)

comment:57 in reply to: ↑ 56 Changed 7 years ago by frasten

Replying to rekkanoryo:

For the record, I nominate sadrul to commit this patch :)

Wow, thank you for the great news!!!

comment:58 Changed 7 years ago by rekkanoryo@…

(In 18ae74c716e6e113cb591a99cefc78204a292102):
Add purple_marshal_POINTER__POINTER(), which sadrul says should be used for the patch attached to #4598. Refs #4598.

comment:59 follow-up: Changed 7 years ago by rekkanoryo

daniele_athome, I need a name with which to credit you for this patch.

comment:60 Changed 7 years ago by rekkanoryo@…

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

(In db6688b33c68fadf19bcad4f6240b476ecfc9496):
Add a new signal, drawing-buddy, to allow plugins to handle markup in buddy names, or anything else a plugin author might choose to dream up. Fixes #4598.

comment:61 in reply to: ↑ 59 Changed 7 years ago by daniele_athome

Replying to rekkanoryo:

daniele_athome, I need a name with which to credit you for this patch.

My name is Daniele Ricci, e-mail is in my trac account.

comment:62 follow-up: Changed 7 years ago by MastroPino

@Daniele Ricci Daniel sto to develope this patch about one year ago. This patch was improved by Andrea Piccinelli aka Frasten, he added more new feature and clean the original code released by daniel-athome. So I think that the name that MUST be on the patch isn't Daniele Ricci but the one of new developer.

A Daniele Ricci, scusami ma a quanto ne sò io, per di questa patch non scrivi una riga da anni e mi sembra proprio da "Italiano" il non citare chi ha portato avanti il lavoro.

comment:63 in reply to: ↑ 62 ; follow-up: Changed 7 years ago by daniele_athome

Replying to MastroPino:

@Daniele Ricci Daniel sto to develope this patch about one year ago. This patch was improved by Andrea Piccinelli aka Frasten, he added more new feature and clean the original code released by daniel-athome. So I think that the name that MUST be on the patch isn't Daniele Ricci but the one of new developer.

I guess rekkanoryo will insert my name together with the other contributors, isn't it rekkanoryo? I didn't talk about you MastroPino? because I thought it was clear that i'm not the main contributor...

[ Si, proprio da italiano... che acidita'... non che me ne importi molto del credit, ho solo risposto alla richiesta, non ho avuto tempo di scrivere altro. ]

comment:64 in reply to: ↑ 63 Changed 7 years ago by MastroPino

Replying to daniele_athome:

Replying to MastroPino:

@Daniele Ricci Daniel sto to develope this patch about one year ago. This patch was improved by Andrea Piccinelli aka Frasten, he added more new feature and clean the original code released by daniel-athome. So I think that the name that MUST be on the patch isn't Daniele Ricci but the one of new developer.

I guess rekkanoryo will insert my name together with the other contributors, isn't it rekkanoryo? I didn't talk about you MastroPino? because I thought it was clear that i'm not the main contributor...

[ Si, proprio da italiano... che acidita'... non che me ne importi molto del credit, ho solo risposto alla richiesta, non ho avuto tempo di scrivere altro. ]

Non è acidità, ma schiettezza, comunque il tempo manca a tutti, difatti non scrivi una riga di codice da almeno 12 mesi!

comment:65 Changed 7 years ago by MastroPino

Anyway I say thanks to Daniel Ricci for having thrown the basis of this patch, but I think the other partner should be mentioned:)

Acid did not want to be!

comment:66 Changed 7 years ago by rekkanoryo

I have credited both Daniele and Andrea for the patch. It should be noted that I did not and will not commit the plugin using this signal to Pidgin's tree--people who want the feature can load a plugin that does what they want. The code I committed does nothing but create and emit a signal for plugin authors to use. If I missed anyone who needs credited, let me know, but keep in mind that only people who contributed to the patch to Pidgin's gtkblist.c get credited.

comment:67 Changed 7 years ago by rekkanoryo@…

(In 79658787f31e562af9d263154475e79d718e2558):
Credit Daniele Ricci and Andrea Piccinelli for the patch I committed earlier. Refs #4598. If I missed anyone, let me know.

comment:68 Changed 18 months ago by dx

Ticket #12418 has been marked as a duplicate of this ticket.

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!