GNOME Bugzilla – Bug 705338
OOO message not spell-checked
Last modified: 2013-11-29 12:15:17 UTC
Please enable spell-checking on the OOO message editor.
In fact, perhaps this should be an *HTML* editor and we should setting the messages as HTML? cf. bug 705337
I'm not in favour for the HTML editor, there is no enough space for it there, but I agree that having a spell-checking there will be nice. There is no spell-checked for GtkTextView currently, or at least I do not know about any, but as I'll need it too, I'll write it one day (we have an ESpellEntry already, based on the libsexy). I'm making this a dependency of bug #329616
Created attachment 260249 [details] [review] Patch adding gtkspell support allowing spell checking for OOO text views Instead of adopting ESpellEntry to an ESpellTextView within evolution-data-server it is much easier/simpler to depend on gtkspell to allow spell checking of text views. In cases the requirement is not wanted it might be possible to import the library functionality, but this would of cause increase the maintenance work. Waiting for comments.
Review of attachment 260249 [details] [review]: Thanks for the patch, it's a very nice start (and pretty simple). I'd like to change/extend some things, though: a) the one in configure (see below) b) the preselected language might not be the one based on the current system locale, but the one chosen in Composer (composer supports multiple languages at once, thus pick one used there) c) the Languages submenu shows en_CA and similar names, instead of user-friendly names, like "English (Canada)" ::: configure.ac @@ +276,3 @@ + [AS_HELP_STRING([--enable-gtkspell], + [Enable gtkspell @<:@default=no@:>@])], + [enable_gtkspell="$enableval"], [enable_gtkspell="no"]) default to 'yes', thus others will notice the new feature
Created attachment 262824 [details] [review] Patch adding gtkspell support allowing spell checking for OOO text views - v2 Based on the review I've changed the configure default value of gtkspell to 'yes'. Furthermore I've retrieved the list of composer mail languages and instruct gtkspell to use the first language in the list. Regarding the "en_US", "de_DE" entries there is nothing we can do about it yet. If this is a serious problem we need to talk to the maintainer of gtkspell.
Review of attachment 262824 [details] [review]: Thanks for an update, this makes it half way where I'd like to have it. We can commit this (though see below), and then enhance the popup menu, for which I expected the gtkspell_get_suggestions_menu() would be used, though the worst case would be to copy the popup menu code from gtkspell and only replace the labels with languages with something more user friendly, similar to [1]. I want that to be done in some reusable way, because it'll be used in evolution itself as well (thus it might be useful to add some generic code into evolution/e-util/ and use it in ews). [1] https://git.gnome.org/browse/evolution/tree/modules/mail/em-composer-prefs.c#n192 ::: src/configuration/e-mail-config-ews-ooo-page.c @@ +590,3 @@ widget = gtk_text_view_new (); +#ifdef WITH_GTKSPELL + gchar **languages = spell_get_languages (); This produces a compiler warning: e-mail-config-ews-ooo-page.c: In function 'mail_config_ews_ooo_page_constructed': e-mail-config-ews-ooo-page.c:592:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] gchar **languages = spell_get_languages (); ^
(In reply to comment #6) > I want that to be done in some reusable way, because it'll be used in > evolution itself as well (thus it might be useful to add some generic > code into evolution/e-util/ and use it in ews). By the way, that would mean that the dependency on gtkspell will be added to evolution, and ews will use only a public e-util API to attach/detach the gtkspell, while not knowing it's gtkspell at all.
Jan-Michael, I would like to commit this on Friday (tomorrow), the initial part (without popup menu customization, I wanted to do that as the next step anyway). Are you working on it, or I can just pick your work and distribute it into evolution and evolution-ews? My idea is to introduce evolution/e-util/e-spell-text-view.c/.h which will provide only simple interface of two functions: typedef void ESpellTextView; ESpellTextView *e_spell_text_view_attach (GtkTextView *text_view); void e_spell_text_view_detach (ESpellTextView *spell); where the ESpellTextView is just a placeholder for GtkSpell, which I do not want to expose outside of the .c file, for cases where evolution would be compiled without gtkspell. The e_spell_text_view_attach() will do what you currently have in the evolution-ews code, including the selected language read from GSettings. In the next step (out of this bug report), it'll also make the language popup menu with user-friendly language names.
Ouch, one thing I didn't notice, you use gtkspell for gtk2, but evolution is gtk3 based application, thus the newer gtkspell should be used. It also seems to provide language names for free, thus one gains two things in once.
Created commit f19bffb in ews master (3.11.3+) All the credits go to Jan-Michael Brummer, because the Evolution interface for GtkTextView spell checker was completely based on his work and idea of using GtkSpell. Note: This requires commit e607ae8 from evolution git master (see bug #329616)