Re: [Evolution-hackers] Bug #127546 Gaim Evolution presence integration.



Thanks for your comments. There is a modified patch which fixes some
of the issues like presence icons. Please have a look at the README
file.

On Wed, 29 Sep 2004 09:15:31 -0700, Christian Hammond <chipx86 gmail com> wrote:
> I have to be honest and say, I really don't like these patches, and
> that's not because you're using my bounty entry to write them ;)
> You're doing way more than you should, and in a way that will make
> Evo's presence integration look and work differently from other
> upcoming desktop apps.
> 
> You duplicate a framework that Galago already provides, and you're
> doing your own widgets and rendering when Galago provides those as
> well. Your patch is 260KB currently, while mine (which does the same
> stuff) is 10KB. I don't think it needs to be so complicated,
> especially when the framework is already provided.
I am totally lost here. By 10K patch do you mean the patches you have
in galago SVN repository because i think i am doing more than that, to
implement whatever is described in the bounty page corresponding to
the bug like presence icons in email entry widget of composer
(gal/e-text/e-text{.c,.h}), presence icons in completion popup
(gal/e-text/e-completion{.c,.h}), presence icons in select names popup
for composer. Contact icon in mail display (em-format-html-display.c),
presence icons in minicard view, addressbook list view , icons in
preview pane of
addressbook. All of them must be clickable and should update
themselves on new presence information. I did not use galago widgets
because for presence icon there is a GtkImage based widget provided by
galago and most of the above widgets use e-table and corresponding
e-cell-* widgets. It can probably be done by writing a custom e-cell
but i will have to look into it. It was far easier to use the existing
e-cell-pixbuf to do the rendering and use existing e-table models to
issue updated presence information. These become essential if i was to
use gaim-remote too as a backend.  Also I simply cannot disregard
gaim-remote, i had asked previously on this list before starting
coding if there were any changes but nobody said anything and hence i
started implementing as described in the bug report. So unless someone
from the evolution team comes and says not to use gaim-remote
specifically, i cannot remove the gaim remote specific code.
For displaying contact photo in mail display/addressbook display,
galago widgets cannot be used anyway and i have to write a custom
widget . There is another custom widget in the sources to display a
popup when clicked on a presence icon. Did you have a really good look
at the code because i don't think i am doing anything unnecessary or
trying to unnecessarily duplicate any existing widgets provided by
galago.

I am using a cache to associate galago accounts with evolution
contacts which can be shared by mail, addressbook, composer parts.
This is done to aggregate 3-4 (such as account-added, account-removed,
account-updated, presence-updated) signal handlers which i need to put
in place for each widget in evolution which needs updates and what
about core-registered, core-unregistered. It also simplifies the
update logic to be implemented for each widget and if you look at the
code you might understand better why i  have taken this approach.It
will be also easier if presence icons need to be put anywhere else
like contact-editor.

This is my first time programming in gtk and i might have definitely
made some mistakes or missed/overlooked something but i am not sure i
agree with you when you say that i have overly complicated this or i
have not tried to use existing frameworks. Please provide specifics if
you still feel that i have seriously messed up.

> 
> Part of what you would get with the widgets are standard icons. Your
> current method will show the AIM icon for every protocol, it seems.
> That's confusing to users when they're looking at MSN contacts.
This has been fixed with the updated patch posted on bugzilla page. 

> some of the Galago stuff you think is broken actually isn't, I just
> don't think you're using the API correctly. You shouldn't be touching
> galago_core_set_watch_all(). That's more of a backend thing.

The problem i faced with galago was - account Y in buddy list of
account X is not updated when account X signs of. The presence
information of account Y is untouched even after account X signs off.
I don't know if it was designed that way but it seemed broken to me or
am i doing something wrong here?. (the patch i have included for
galago tries to fix this).

 
> Is there still need for a dependency on Gaim (which will cause
> Evolution to break more often than not, since we're not guaranteeing
> any API stability at the moment) when you're working on Galago
> support?
The dependency on gaim is present because on the bounty page it is
mentioned that a popup should be presented by which you can open a
conversation window with a contact to chat. That is why the dependency
on gaim is present.

By the way, i have made gaim optional in the updated patches. By
default gaim-remote stuff  (libevogaimremote.so) is not
compiled/installed but dependency on gaim is there to open
conversation window. In this case gaim patches need not be applied.
(configure option --enable-gaim=urionly)

If you want to use only galago, gaim patches need not be applied, in
that case you can't send chat requests from evolution (use configure
option --enable-gaim=no)

Praveen.

> 
> Christian
> 
> 
> 
> On Wed, 29 Sep 2004 11:23:25 -0400, praveen kandikuppa
> <praveen9 gmail com> wrote:
> > I have posted an updated patch for this bug at
> > http://bugzilla.gnome.org/show_bug.cgi?id=127546. Please have a look
> > at the README file in the attachment. The main new feature is now
> > galago (http://galago.sourceforge.net/) can be used as one of the
> > backends for querying presence information and in my opinion it would
> > be better to have it as default, it is im client independent and has
> > instantaneous updates on signoff/signon etc.
> >
> > PObject is used now while displaying presence icons in mail display.
> > there are various code cleanups and less hacks and it should be a lot
> > more readable now. the README file describes some more changes.
> >
> > In my opinion the patch implements most of the bug and sans some minor
> > changes should be ready. Whom do i contact/mail to get the changes
> > reviewed/accepted. I cannot see any owner associated The patch is
> > little big to be mailed to evolution-patches. Can i mail a bzip2 file
> > to the patches list?.
> >
> > Praveen.
> >
> > 
> > _______________________________________________
> > evolution-hackers maillist  -  evolution-hackers lists ximian com
> > http://lists.ximian.com/mailman/listinfo/evolution-hackers
> >
>



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]