There is lack of the method in TextCheckerEnchant class which allows the client to get the list of all available/installed dictionaries. To set the dictionaries the user have to 'guess/hardcode' the names of dictionaries that might be non obvious. The list can be used to set dictionaries used by Enchant. To avoid names conflict this patch proposed a new one for former getSpellCheckingLanguages. It has been changed to loadedSpellCheckingLanguages as it gets the current (in use) dictionaries. Additionally the 'get' prefix has been removed according to WebKit coding style.
Created attachment 163795 [details] proposed patch This getter can be used by WebKit's GTK and EFL API for instance: - webkit_web_context_set_spell_checking_languages(), - ewk_text_checker_setting_spell_checking_languages_set() to set dictionaries used by Enchant.
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment on attachment 163795 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=163795&action=review The patch looks good to me, although I'm a bit confused because I was expecting to see this new function used in the patch for bug 91854, but I guess that will come in a separate patch? Otherwise, I just have one humble suggestion (see below). > Source/WebCore/platform/text/enchant/TextCheckerEnchant.cpp:196 > + enchant_broker_list_dicts(m_broker, enchantDictDescribeCallback, &allDictionaries); Maybe early return if allDictionaries.isEmpty()?
(In reply to comment #3) > (From update of attachment 163795 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=163795&action=review > > The patch looks good to me, although I'm a bit confused because I was expecting to see this new function used in the patch for bug 91854, but I guess that will come in a separate patch? The last patch for bug 91854 adds ewk_text_checker_setting_spell_checking_languages_get() which gets already loaded dictionaries not available once that this patch proposes. We decided to keep old API as it is and add a new one ewk_text_checker_setting_available_spell_checking_languages_get() to get installed languages. > > Otherwise, I just have one humble suggestion (see below). > > > Source/WebCore/platform/text/enchant/TextCheckerEnchant.cpp:196 > > + enchant_broker_list_dicts(m_broker, enchantDictDescribeCallback, &allDictionaries); > > Maybe early return if allDictionaries.isEmpty()? I thought about it too. IMO this method is very short and if the allDictionaries.isEmpty() the iterations won't happen of course and method returns an empty vector. To early return we have to move 'Vector<String> languages' somewhere above. If you prefer early return here just let me know I will change it. Thanks.
(In reply to comment #4) > [...] > The last patch for bug 91854 adds ewk_text_checker_setting_spell_checking_languages_get() which gets already loaded dictionaries not available once that this patch proposes. > We decided to keep old API as it is and add a new one ewk_text_checker_setting_available_spell_checking_languages_get() to get installed languages. Ok. Thanks for the clarification. > [...] > > Maybe early return if allDictionaries.isEmpty()? > I thought about it too. IMO this method is very short and if the allDictionaries.isEmpty() the iterations won't happen of course and method returns an empty vector. To early return we have to move 'Vector<String> languages' somewhere above. If you prefer early return here just let me know I will change it. Thanks. Not a big deal. It already looks good to me as it is now.
Comment on attachment 163795 [details] proposed patch I think this is the way to go for both gtk and efl ports.
Comment on attachment 163795 [details] proposed patch LGTM, thanks!
Comment on attachment 163795 [details] proposed patch Clearing flags on attachment: 163795 Committed r128567: <http://trac.webkit.org/changeset/128567>
All reviewed patches have been landed. Closing bug.