GNOME Bugzilla – Bug 532832
Spelling propositions when multi language support
Last modified: 2011-08-29 10:12:43 UTC
I ask Empathy to check the spelling in English and French. Now, in a french speaking chat, the spelling propositions (right button on a red word) Empathy proposes in the same time propositions in English and in French. As example, the spelling propositions for "deformatino" are (amon others): deformation déformation (notice the accent : this is a French proposition) deforming ... It would be better to select a "default" language for spelling correction, and to be able to change it in each single conversation windows. Even better : the possibility to select a prefered language for each contact. Thank for all
I often use English words inside a French conversation. I think it would be great to just split propositions for each language, like xchat-gnome does.
Most easy would just be to remove gtk_tree_view_column_set_sort_column_id (column, COL_SPELL_WORD) in empathy-spell-dialog.c; so that suggestions will be sorted by languages (it would miss a separator between languages, but this is harder to do). Also I like the way xchat-gnome uses a submenu instead of a dialog to provide suggestions.
Strong +1 do to it as xchat-gnome.
Created attachment 171070 [details] [review] proposed fix
Thanks for the patch. I totally want it for 3.0. I'll review it soonish.
Review of attachment 171070 [details] [review]: Hey, sorry for the late review. It looks pretty good! I inlined some comments. I think it would be good to unsensitive langage sub menu which don't have any proposition. ::: empathy-2.31.92/libempathy-gtk/empathy-chat.c @@ +1782,3 @@ + + for (l = suggestions; l; l = l->next) { + menu_item = gtk_menu_item_new_with_label (l->data); This block should be aligned. ::: empathy-2.31.92/libempathy-gtk/empathy-spell.c @@ +51,2 @@ static GHashTable *iso_code_names = NULL; +static GHashTable *languages = NULL; Please add a comment explalining the the type and semantic of the keys and the values. @@ +307,3 @@ + codes = g_list_append (codes, g_strdup (code)); + } + You can use g_hash_table_get_keys(). Note that then you won't have to free the keys any more (just the list).
Created attachment 172292 [details] [review] updated patch
Review of attachment 172292 [details] [review]: You didn't make the change to unsensitive the langage menu if there is no suggestion. So instead of having: English -> (No Suggestion) just insensitive "English". I hit this crash while testing your patch. To reproduce: - Type something - Righ click on it to open the spell menu - Change your spelling settings (add or remove a lang) - Right click on the word again ERROR:empathy-chat.c:1807:chat_spelling_build_menu: assertion failed: (codes != NULL) Program received signal SIGABRT, Aborted. 0x00007fffeeccdba5 in raise (sig=<value optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 64 ../nptl/sysdeps/unix/sysv/linux/raise.c: Aucun fichier ou dossier de ce type. in ../nptl/sysdeps/unix/sysv/linux/raise.c (gdb) bt
+ Trace 224150
::: empathy-2.32.0.1/libempathy-gtk/empathy-chat.c @@ +1804,3 @@ + GList *codes, *l; + + codes = empathy_spell_get_enabled_language_codes (); You leak this list. You have to free it using g_list_free() when you're done. @@ +1831,2 @@ } + empathy_spell_free_language_codes (codes); alignement is wrong.
+ Trace 224151
Created attachment 172379 [details] [review] updated updated patch
I've updated the patch in accordance with your notes. >You didn't make the change to unsensitive the langage menu if there is no suggestion. Sorry, I forgot about this. >I hit this crash while testing your patch. This crash was caused by another spelling-related issue in the original code: We were enabling spell checker each time preferences got initialized (even when there were no enabled languages). See the changes in empathy-preferences.c for details.
Yeah, merged to master \o/ Thanks a lot for your work. This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.