Just notifying Ettore & Anna.

Created attachment 41102 [details] [review]
Patch to allow user to select editor component in mail config

I've attached a patch which implements this functionality.  I've set
the target milestone to 1.2, since I'm hoping it will be merged in for
that release (it shouldn't take much work).  

I'll be happy to help resolve any issues that might arise.

a couple stylistic comments on the patch:

1. we prefer: func (arg) instead of func( arg )

2. we want 8-space tabs. Prefarably these will be actual tabs and not
8-spaces, but either will do.

3. when declaring variables, use: char *var; and not char * var;

4. We mailer guys (Michael Zucchi and I) prefer 'char' rather than
'gchar'
and 'int' rather than 'gint'. guint32's are fine though if you are
needing
a 32bit unsigned int.
(I noticed you used gchar in a few places, but it looked like you used
ints rather than gints)

The reason for using int and char rather than gint and gchar is that
emacs will do syntax highlighting without any extra configuring which
makes code much easier to read no matter what system.


Other comments that I have are that you forgot to add
editor-components.[c,h] ;-)

Oh, and get_editor_components () doesn't follow any namespacing. It
would be preferable if it was more like:

editor_components_get () or maybe editor_components_get_oafiid_list ()
or whatever (I'm assuming it gets an oafiid list? maybe it doesn't)

That said, once the e-msg-composer.c code gets the stylistic fixes, it
looks okay to me. I can't easily judge the rest without first seeing
editor-components.c though.

Created attachment 41113 [details]
tarball containing updated patch, editor-components.c and editor-components.h

Thanks for the quick response.  Sorry I didn't get back with this
sooner, but I did a cvs update and it took me a few days to rebuild
the world. :)

This attached tarball includes the two missing files (I couldn't find
a way to convince diff to include them in the patch itself, since they
haven't been cvs added).

I've modified my changes to conform to the style guidelines (tabs,
variable naming, etc.)

Please let me know if there's anything else to be done.

*** bug 226426 has been marked as a duplicate of this bug. ***

what are we doing about this?

Adding keyword.

We need copyright assignment, or an intent to provide it, then we can
put it in.

I'm happy to assign copyright of the patch to whomever it needs to be
assigned to (Ximian?).  Does it suffice for me to simply declare this,
or do I need to do something else?

Sorry about the delay, I'll try and get this moving again.

We have a form you need to sign and send to us; I'll email you the
details.

Looks like reporter has moved to jason@alumni.uwaterloo.ca, will try
sending mail there.

Both email addresses work; sorry for the late reply (I've been on
holidays).  I've received the form and will try to get send it in the
next few days.

this is regretably going to have to be punted from 1.2 as we are long
past freeze.

What's going on with this patch? It doesn't seem to apply to 1.2. Can
it be brought up to date and added?

Thanks

Matt Taggart
matt@lackof.org

The ball is in my court -- I got busy and didn't get back to this for
a long time.  In recent months, however, I've been working on adding
proper bonobo support for vim, and will soon be working on the
alternate-editors-for-Evolution front again.
                                                                     
          
Check back for an update to this patch within the next few weeks.
Hopefully the timing will be right to get this merged in for the
post-1.4 dev cycle (once I finally fill out the copyright assignment
form. :)

BTW, while having configurable editor support sounds like a nice thing
in principle, it might be preferable to not expose this setting in the
UI and leave it up to a GConf key.

Hi Guys. 

Jason, thanks for your hard work and diligence. :) 

My preference (as the person responsible for Evolution's UI/usability)
is to leave this settings out of the settings dialog. 

My reasons are these: 

1) Ximian really can't support* alternate composers-- people who want
to use a non-standard composer will be doing so at their own risk. I
don't want to give people the mistaken impression that they can select
a 3rd party composer -and- that we (ximian) will be able to fix any
problems they find it to have. It would be rather unfortunate if we
mislead our users on this point-- and I think that including the
setting in the settings dialog implies that it is supported.

2) I believe that it is important to limit the number of non-essential
settings in the settings dialog, for the sake of keeping the dialog
navigable as the project grows. 

This particular settings strikes me as being: 1) something that few
people will ever change, 2) something that people who want to use a
custom composer will set once and then never change again, 3)
something which does not effect whether or not one can accomplish
basic tasks with Evolution. With those things in mind, it seems like a
good candidate for a gconf key. 

Does that seem reasonable, interested parties? 

-Anna

*By "support" I mean  "provide tech support for/fix bugs in"

Hi Anna,

While I would like to make it as easy as possible for users to
configure an alternate editor, I think most users who'd use this are
capable of tweaking a gconf key.

The thing is, without a visible setting, these users wouldn't even
know that such a thing is possible.  This is kind of sad, as there
will be thousands of people who would use it (simply judging from my
website traffic and emails I receive asking when this will be ready).

But I can appreciate your second reason (keep settings dialogs sane),
and so leaving it as a gconf key will be ok.  I'll just have to
announce the feature on slashdot or something. ;)

cheers,
Jason

Created attachment 43209 [details] [review]
Patch to make editor configurable using gconf key

I'm not sure this has to go thru evolution-patches mailing list now.

Any updates on this one? (Is there a signed copyright assignment? Does the patch apply to HEAD? Can we get this for GNOME 2.16?)

Yes, I signed a copyright assignment form back before Ximian was purchased by Novell, and assume someone somewhere still has it. :)

I tried this about a year and a half ago and didn't get anywhere. I don't really know why, but it seemed to me that fejj either wasn't interested in merging it in, or was too busy with other work to reply to my emails:  

http://www.opensky.ca/~jdhildeb/blog/539_Status_of_the_Evolution_patch.item

I'd be happy to update the patch if there is a good chance it will get committed this time.

Jason, sure. Update the patch. I would be happy to take it in with gconf level setting.

Ok, it will probably take me a few weeks to find time, but I will work at updating the patch to evolution head.

Any updates? (I know feature freeze is already there again, but if we have a patch now, we could discuss/improve it and hopefully get it committed just after the 2-16-branch)

Sorry; I've been swamped with other things.  If I update the patch I'll be sure to submit it here right aways.  

If anyone else wants to jump in and do this, my most recent patch is against 2.2.2, and is available from here:

   http://www.opensky.ca/~jdhildeb/software/gnome-vim/evo-patches/

I don't know what has all changed in the evo codebase since 2.2.2, but I'm expecting it will be relatively straightforward to update the patch to CVS head.

http://git.gnome.org/browse/evolution/tree/plugins/external-editor is available nowadays - does this fulfil the needs to close this report, or what is missing here?

It's a different approach than the patch above, but probably fulfills the needs for most people.

I'm not personally using an external editor with Evo anymore so I will close this ticket.