Merge lp:~sil2100/ubuntu-keyboard/enhance_word_completion into lp:ubuntu-keyboard

Proposed by Łukasz Zemczak
Status: Merged
Approved by: Bill Filler
Approved revision: 168
Merged at revision: 175
Proposed branch: lp:~sil2100/ubuntu-keyboard/enhance_word_completion
Merge into: lp:ubuntu-keyboard
Diff against target: 473 lines (+211/-25)
13 files modified
plugins/pinyin/src/chineselanguagefeatures.cpp (+19/-1)
plugins/pinyin/src/chineselanguagefeatures.h (+1/-0)
plugins/westernsupport/westernlanguagefeatures.cpp (+15/-0)
plugins/westernsupport/westernlanguagefeatures.h (+2/-0)
qml/Keyboard.qml (+1/-0)
qml/KeyboardContainer.qml (+2/-0)
qml/keys/CharKey.qml (+8/-1)
src/lib/logic/abstractlanguagefeatures.h (+2/-0)
src/view/abstracttexteditor.cpp (+94/-16)
src/view/abstracttexteditor.h (+1/-0)
tests/unittests/ut_editor/ut_editor.cpp (+29/-4)
tests/unittests/ut_editor/wordengineprobe.cpp (+32/-0)
tests/unittests/ut_editor/wordengineprobe.h (+5/-3)
To merge this branch: bzr merge lp:~sil2100/ubuntu-keyboard/enhance_word_completion
Reviewer Review Type Date Requested Status
Bill Filler (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
Michael Sheldon (community) Needs Fixing
Review via email: mp+214996@code.launchpad.net

Commit message

Changes related to fixing the preedit word completion, enhancing the usage with regards to adding whitespaces, separators. Enhanced autocaps. Added possibility to commit (word-completion) when using separators (., etc). Double-space with auto-correction enabled now results in a full-stop sign. Also, in auto-correct unnecessary whitespaces before separators are removed. Include unit-testing of some of the features.

Description of the change

* Are there any related MPs required for this MP to build/function as expected? Please list.
-> No

* Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)
-> Yes

* Did you perform an exploratory manual test run of your code change and any related functionality on device or emulator?
-> Yes, on my mako device

* Did you successfully run all tests found in your component's Test Plan on device or emulator?
-> Yes, on my mako device

* If you changed the UI, was the change specified/approved by design?
-> N/A

* If you changed the packaging (debian), did you subscribe a core-dev to this MP?
-> N/A

Description:

Changes related to fixing the preedit word completion, enhancing the usage with regards to adding whitespaces, separators. Enhanced autocaps.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

One slight issue is the spacing behaviour when using Pinyin. At the moment you have to do a double space to get a space character for Pinyin (and then of course you get two spaces). It behaves correctly when entering characters, in that it's not appending a space after completing a character, but when entering a space on its own (after finishing a character) you currently don't get any output.

153. By Łukasz Zemczak

Enable the double-space as a full stop sequence generator during auto-complete

154. By Łukasz Zemczak

Remove any trailing whitespaces during auto-correct when a separator is inputted and make the double-space action only work in case of space-started auto-correct

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

I will look now into the issue with Pinyin - currently it seems to be not working at all even (maliit-server stops due to assertion issues). In the meantime, just a quick comment related to the removal of whitespaces that I made here. The best, cleanest way would be to use preedit for those, but sadly that approach was very very troublesome. Preedit has a specific way of formatting its input. And it also interferes with the word-prediction... Since it was really really troublesome, I decided to use a more 'hacky way' for this - doing backspace actions to remove the whitespaces. It's not perfect, but it seems to be the only sane way without having to modify maliit too much.

155. By Łukasz Zemczak

Fix the new behavior for pinyin

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
156. By Łukasz Zemczak

Add some additional comments

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
157. By Łukasz Zemczak

Add the possibility to commit strings using separators (like dots and commas). Fix pinyin input to consider separators

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
158. By Łukasz Zemczak

Merge trunk

159. By Łukasz Zemczak

Fix the tests, still working on fixing all edge-cases

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
160. By Łukasz Zemczak

Fix all outstanding issues... Refactor the code heavily as it was a 'patch on a patch of a patch'. Now it should be good

161. By Łukasz Zemczak

Merge trunk

162. By Łukasz Zemczak

Add a new unit test for the double-space full-stop

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

I'm finding that when first entering an empty field caps don't get disabled until the second letter has been completed, so "Hello" becomes "HEllo". This seems to be related to the panel.autoCapsTriggered condition on line 114 of the diff, which prevents the keypad state from being set back to NORMAL. With that removed everything appears to work correctly, however I'm not clear on what the intention is there so I might be missing some side effect that I'm unaware of?

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Sadly, this conditional is required to enable activation of autocaps by normal character key-presses. Without this, each character key automatically disables autocaps, making it impossible to triggering it properly. I just need to handle this case better, let me fix this ;)

163. By Łukasz Zemczak

Workaround fix for the broken initial autocaps

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?

 * Yes

Did CI run pass? If not, please explain why.

 * Yes

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?

 * Yes

review: Approve
Revision history for this message
Bill Filler (bfiller) wrote :

Getting close. There are still a few issues that need to be addressed:

For all of these tests, I just open Notes and type a new note

== turn on all the settings (spellcheck, autocomplete, word suggestions)
1) No space is entered after pressing spacebar to commit the word. For example type:
"Hey my"[spacebar]
This should produce
"Hey my " but instead produces "Hey my"
The space does not get added until you type the next word. Whenever spacebar is pressed it should commit the word and add a space.

2) Double tap on spacebar not always adding punctation:
This works correctly if there is a suggested word in the list to complete. i.e
"Hello my fri" [double tap spacebar] produces "Hello my friend. "
But if there is no suggested word, or if word suggestion/auto complete is disabled it does not work.
For example:
"Hello my uubgugugugug"[double tap spacebar] produces "Hello my uubgugugugug " while it should produce "Hello my uubgugugugug.

The rule should be double-tapping spacebar when the preceding character is not a space should always a) commit the word b) add the period + trailing space, regardless if word suggesting existed or not and regardless if word suggestion/auto-complete is enabled. This is how Android and iphone both work

review: Needs Fixing
164. By Łukasz Zemczak

Change the approach: for now let's not use the preedit for appending the whitespaces for auto-correct. Let's actually print it out but remove it when needed. We could do this otherwise, i.e. use preedit and custom formatting strings, but that actually requires more work and is more error prone. Let's do that later. The unit tests need some love still though...

165. By Łukasz Zemczak

Merge trunk

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

As per our earlier discussion, Android doesn't enable double-space = full-stop when auto-complete is disabled. I guess most of the things mentioned should be resolved.

As already mentioned in the trunk commits, now we're doing things differently. No more usage of preedit. It's less clean, but much less error prone. I would consider using preedit again with custom formatting strings, but let's leave that for some later refactoring :)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

Spotted one new issue, if you enter a full stop when in a URL/email context (e.g. in the browser address bar or the messaging app email field) a space is now inserted, this behaviour should probably be disabled under these circumstances.

Revision history for this message
Michael Sheldon (michael-sheldon) :
review: Needs Fixing
Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

I think we need to be more picky over the punctuation behaviour as well, it currently causes issues when inserting smilies in text. For example, if you try typing "This is getting better :)" you end up with "This is getting better: )"

review: Needs Fixing
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Ah! Smileys ;) Forgot about those! Let me make a quick fix soon.

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Just a quick heads up - I'm working on this, just have a lot of landing team work nowadays. The fix for those issues is almost done, just need to find 10 free minutes.

166. By Łukasz Zemczak

Do not consider : and ; as separators right now

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
167. By Łukasz Zemczak

Yeah, this should be much easier to do the detection of content type - as all non free text content types explicitly disable preedit.

168. By Łukasz Zemczak

Merge trunk

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Should be ok now ;p! Curse you landing team work!

btw. I tried connecting to the contentType changed signals, but those didn't really work. But this way should cover all the currently known cases as well.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Bill Filler (bfiller) wrote :

I've tested out of silo 3, it's all working nicely. Good job Sil!

Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?
yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/<package-name>) on device or emulator?
yes

Did CI run pass? If not, please explain why.
yes

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?
yes

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/pinyin/src/chineselanguagefeatures.cpp'
2--- plugins/pinyin/src/chineselanguagefeatures.cpp 2014-04-15 12:18:47 +0000
3+++ plugins/pinyin/src/chineselanguagefeatures.cpp 2014-06-11 13:16:05 +0000
4@@ -47,6 +47,24 @@
5
6 QString ChineseLanguageFeatures::appendixForReplacedPreedit(const QString &preedit) const
7 {
8- Q_UNUSED(preedit)
9+ if (isSeparator(preedit.right(1))) {
10+ return QString(" ");
11+ }
12+
13 return QString("");
14 }
15+
16+bool ChineseLanguageFeatures::isSeparator(const QString &text) const
17+{
18+ static const QString separators = QString::fromUtf8("。、,!?:;\r\n");
19+
20+ if (text.isEmpty()) {
21+ return false;
22+ }
23+
24+ if (separators.contains(text.right(1))) {
25+ return true;
26+ }
27+
28+ return false;
29+}
30\ No newline at end of file
31
32=== modified file 'plugins/pinyin/src/chineselanguagefeatures.h'
33--- plugins/pinyin/src/chineselanguagefeatures.h 2014-04-15 12:18:47 +0000
34+++ plugins/pinyin/src/chineselanguagefeatures.h 2014-06-11 13:16:05 +0000
35@@ -31,6 +31,7 @@
36 virtual bool autoCapsAvailable() const;
37 virtual bool activateAutoCaps(const QString &preedit) const;
38 virtual QString appendixForReplacedPreedit(const QString &preedit) const;
39+ virtual bool isSeparator(const QString &text) const;
40 };
41
42 #endif // CHINESELANGUAGEFEATURES_H
43
44=== modified file 'plugins/westernsupport/westernlanguagefeatures.cpp'
45--- plugins/westernsupport/westernlanguagefeatures.cpp 2014-04-15 12:18:47 +0000
46+++ plugins/westernsupport/westernlanguagefeatures.cpp 2014-06-11 13:16:05 +0000
47@@ -74,3 +74,18 @@
48
49 return QString(" ");
50 }
51+
52+bool WesternLanguageFeatures::isSeparator(const QString &text) const
53+{
54+ static const QString separators = QString::fromUtf8(",.!?\r\n");
55+
56+ if (text.isEmpty()) {
57+ return false;
58+ }
59+
60+ if (separators.contains(text.right(1))) {
61+ return true;
62+ }
63+
64+ return false;
65+}
66
67=== modified file 'plugins/westernsupport/westernlanguagefeatures.h'
68--- plugins/westernsupport/westernlanguagefeatures.h 2014-04-15 12:18:47 +0000
69+++ plugins/westernsupport/westernlanguagefeatures.h 2014-06-11 13:16:05 +0000
70@@ -46,6 +46,8 @@
71 virtual bool autoCapsAvailable() const;
72 virtual bool activateAutoCaps(const QString &preedit) const;
73 virtual QString appendixForReplacedPreedit(const QString &preedit) const;
74+ virtual bool isSeparator(const QString &text) const;
75+ virtual QString fullStopSequence() const { return QString("."); }
76 };
77
78 #endif // MALIITKEYBOARD_LANGUAGEFEATURES_H
79
80=== modified file 'qml/Keyboard.qml'
81--- qml/Keyboard.qml 2014-02-28 18:25:13 +0000
82+++ qml/Keyboard.qml 2014-06-11 13:16:05 +0000
83@@ -244,6 +244,7 @@
84 onActivateAutocaps: {
85 keypad.state = "CHARACTERS";
86 keypad.activeKeypadState = "SHIFTED";
87+ keypad.autoCapsTriggered = true;
88 }
89 }
90
91
92=== modified file 'qml/KeyboardContainer.qml'
93--- qml/KeyboardContainer.qml 2014-05-12 20:24:36 +0000
94+++ qml/KeyboardContainer.qml 2014-06-11 13:16:05 +0000
95@@ -28,6 +28,8 @@
96 property int keyWidth: 0
97 property int keyHeight: 0
98
99+ property bool autoCapsTriggered: false
100+
101 property string activeKeypadState: "NORMAL"
102 property alias popoverEnabled: extendedKeysSelector.enabled
103
104
105=== modified file 'qml/keys/CharKey.qml'
106--- qml/keys/CharKey.qml 2014-05-01 16:39:00 +0000
107+++ qml/keys/CharKey.qml 2014-06-11 13:16:05 +0000
108@@ -142,9 +142,14 @@
109 onReleased: {
110 if (!extendedKeysShown) {
111 event_handler.onKeyReleased(valueToSubmit, action);
112- if (!skipAutoCaps)
113+
114+ if (panel.autoCapsTriggered) {
115+ panel.autoCapsTriggered = false;
116+ }
117+ else if (!skipAutoCaps) {
118 if (panel.activeKeypadState === "SHIFTED" && panel.state === "CHARACTERS")
119 panel.activeKeypadState = "NORMAL"
120+ }
121 }
122 }
123 onPressed: {
124@@ -154,6 +159,8 @@
125 if (maliit_input_method.useHapticFeedback)
126 pressEffect.start();
127
128+ // Quick workaround to fix initial autocaps - not beautiful, but works
129+ panel.autoCapsTriggered = false;
130 event_handler.onKeyPressed(valueToSubmit, action);
131 }
132 }
133
134=== modified file 'src/lib/logic/abstractlanguagefeatures.h'
135--- src/lib/logic/abstractlanguagefeatures.h 2014-04-15 12:19:05 +0000
136+++ src/lib/logic/abstractlanguagefeatures.h 2014-06-11 13:16:05 +0000
137@@ -45,6 +45,8 @@
138 virtual bool autoCapsAvailable() const = 0;
139 virtual bool activateAutoCaps(const QString &preedit) const = 0;
140 virtual QString appendixForReplacedPreedit(const QString &preedit) const = 0;
141+ virtual bool isSeparator(const QString &text) const { Q_UNUSED(text); return false; }
142+ virtual QString fullStopSequence() const { return QString(); }
143 };
144
145 #endif // MALIIT_KEYBOARD_ABSTRACTLANGUAGEFEATURES_H
146
147=== modified file 'src/view/abstracttexteditor.cpp'
148--- src/view/abstracttexteditor.cpp 2014-05-27 15:37:37 +0000
149+++ src/view/abstracttexteditor.cpp 2014-06-11 13:16:05 +0000
150@@ -285,6 +285,8 @@
151 bool auto_caps_enabled;
152 int ignore_next_cursor_position;
153 QString ignore_next_surrounding_text;
154+ bool look_for_a_double_space;
155+ QString appendix_for_previous_preedit;
156 int backspace_word_acceleration;
157
158 explicit AbstractTextEditorPrivate(const EditorOptions &new_options,
159@@ -307,6 +309,8 @@
160 , auto_caps_enabled(false)
161 , ignore_next_cursor_position(-1)
162 , ignore_next_surrounding_text()
163+ , look_for_a_double_space(false)
164+ , appendix_for_previous_preedit()
165 , backspace_word_acceleration(0)
166 {
167 auto_repeat_backspace_timer.setSingleShot(true);
168@@ -415,23 +419,66 @@
169 const QString text = key.label();
170 QString keyText = QString("");
171 Qt::Key event_key = Qt::Key_unknown;
172+ bool look_for_a_double_space = d->look_for_a_double_space;
173+
174+ if (look_for_a_double_space) {
175+ // we reset the flag here so that we won't have to add boilerplate code later
176+ d->look_for_a_double_space = false;
177+ }
178
179 switch(key.action()) {
180 case Key::ActionInsert: {
181- d->text->appendToPreedit(text);
182+ bool alreadyAppended = false;
183+ bool auto_caps_activated = false;
184+ const bool isSeparator = d->word_engine->languageFeature()->isSeparator(text);
185+ const bool replace_preedit = d->auto_correct_enabled && not d->text->primaryCandidate().isEmpty() &&
186+ not d->text->preedit().isEmpty() && isSeparator;
187
188- // computeCandidates can change preedit face, so needs to happen
189- // before sending preedit:
190 if (d->preedit_enabled) {
191- d->word_engine->computeCandidates(d->text.data());
192- }
193-
194- sendPreeditString(d->text->preedit(), d->text->preeditFace(),
195- Replacement(d->text->cursorPosition()));
196+ if (replace_preedit) {
197+ // this means we should commit the candidate, add the separator and whitespace
198+ d->text->setPreedit(d->text->primaryCandidate());
199+ d->text->appendToPreedit(text);
200+ d->appendix_for_previous_preedit = d->word_engine->languageFeature()->appendixForReplacedPreedit(d->text->preedit());
201+ d->text->appendToPreedit(d->appendix_for_previous_preedit);
202+ commitPreedit();
203+ auto_caps_activated = d->word_engine->languageFeature()->activateAutoCaps(d->text->surroundingLeft() + d->text->preedit() + text);
204+ alreadyAppended = true;
205+ }
206+ else if (d->auto_correct_enabled && isSeparator) {
207+ // remove all whitespaces before the separator, then add a whitespace after it
208+ removeTrailingWhitespaces();
209+
210+ d->text->appendToPreedit(text);
211+ auto_caps_activated = d->word_engine->languageFeature()->activateAutoCaps(d->text->surroundingLeft() + d->text->preedit());
212+ d->text->appendToPreedit(d->word_engine->languageFeature()->appendixForReplacedPreedit(d->text->preedit()));
213+ commitPreedit();
214+ alreadyAppended = true;
215+ }
216+ }
217+
218+ // if we had modified the preedit already because of a separator entry, there is no need to perform all the
219+ // steps like appending the input or computing candidates - as all we needed was already done in the previous part
220+ if (not alreadyAppended) {
221+ d->text->appendToPreedit(text);
222+
223+ // computeCandidates can change preedit face, so needs to happen
224+ // before sending preedit:
225+ if (d->preedit_enabled) {
226+ d->word_engine->computeCandidates(d->text.data());
227+ }
228+
229+ sendPreeditString(d->text->preedit(), d->text->preeditFace(),
230+ Replacement(d->text->cursorPosition()));
231+ }
232
233 if (not d->preedit_enabled) {
234 commitPreedit();
235 }
236+ else if (auto_caps_activated && d->auto_caps_enabled) {
237+ // standard input (like certain separators, e.g. '.' in western languages) can also trigger autoCaps
238+ Q_EMIT autoCapsActivated();
239+ }
240 } break;
241
242 case Key::ActionBackspace: {
243@@ -445,17 +492,31 @@
244 } break;
245
246 case Key::ActionSpace: {
247+ QString space = " ";
248 QString textOnLeft = d->text->surroundingLeft() + d->text->preedit();
249- const bool auto_caps_activated = d->word_engine->languageFeature()->activateAutoCaps(textOnLeft);
250+ bool auto_caps_activated = d->word_engine->languageFeature()->activateAutoCaps(textOnLeft);
251 const bool replace_preedit = d->auto_correct_enabled && not d->text->primaryCandidate().isEmpty() && not d->text->preedit().isEmpty();
252+ const QString stopSequence = d->word_engine->languageFeature()->fullStopSequence();
253+
254+ // every double-space character inputs one-after-another force a full-stop, so trigger it if needed
255+ if (d->preedit_enabled && d->auto_correct_enabled && not look_for_a_double_space) {
256+ d->look_for_a_double_space = true;
257+ }
258
259 if (replace_preedit) {
260- const QString &appendix = d->word_engine->languageFeature()->appendixForReplacedPreedit(d->text->preedit());
261+ space = d->appendix_for_previous_preedit = d->word_engine->languageFeature()->appendixForReplacedPreedit(d->text->preedit());
262 d->text->setPreedit(d->text->primaryCandidate());
263- d->text->appendToPreedit(appendix);
264- } else {
265- d->text->appendToPreedit(" ");
266- }
267+ }
268+ else if (look_for_a_double_space && not stopSequence.isEmpty()) {
269+ removeTrailingWhitespaces();
270+ d->text->appendToPreedit(d->word_engine->languageFeature()->fullStopSequence());
271+
272+ // we need to re-evaluate autocaps after our changes to the preedit
273+ textOnLeft = d->text->surroundingLeft() + d->text->preedit();
274+ auto_caps_activated = d->word_engine->languageFeature()->activateAutoCaps(textOnLeft);
275+ }
276+
277+ d->text->appendToPreedit(space);
278 commitPreedit();
279
280 if (auto_caps_activated && d->auto_caps_enabled) {
281@@ -578,8 +639,10 @@
282
283 d->text->setPreedit(replacement);
284 const bool auto_caps_activated = d->word_engine->languageFeature()->activateAutoCaps(d->text->preedit());
285- const QString appendix = d->word_engine->languageFeature()->appendixForReplacedPreedit(d->text->preedit());
286- d->text->appendToPreedit(appendix);
287+ d->appendix_for_previous_preedit = d->word_engine->languageFeature()->appendixForReplacedPreedit(d->text->preedit());
288+ if (d->auto_correct_enabled) {
289+ d->text->appendToPreedit(d->appendix_for_previous_preedit);
290+ }
291 commitPreedit();
292
293 if (auto_caps_activated && d->auto_caps_enabled) {
294@@ -674,6 +737,21 @@
295 d->word_engine->clearCandidates();
296 }
297
298+void AbstractTextEditor::removeTrailingWhitespaces()
299+{
300+ Q_D(AbstractTextEditor);
301+
302+ const QString textOnLeft = d->text->surroundingLeft() + d->text->preedit();
303+
304+ QString::const_iterator begin = textOnLeft.cbegin();
305+ QString::const_iterator i = textOnLeft.cend();
306+ while (i != begin) {
307+ --i;
308+ if (*i != ' ') break;
309+ singleBackspace();
310+ }
311+}
312+
313 // TODO: this implementation does not take into account following features:
314 // 1) preedit string
315 // if there is preedit then first call to autoRepeatBackspace should clean it completely
316
317=== modified file 'src/view/abstracttexteditor.h'
318--- src/view/abstracttexteditor.h 2014-05-14 13:14:34 +0000
319+++ src/view/abstracttexteditor.h 2014-06-11 13:16:05 +0000
320@@ -158,6 +158,7 @@
321 virtual void singleBackspace();
322
323 void commitPreedit();
324+ void removeTrailingWhitespaces();
325 Q_SLOT void autoRepeatBackspace();
326 void autoRepeatWordBackspace();
327 QString wordLeftOfCursor() const;
328
329=== modified file 'tests/unittests/ut_editor/ut_editor.cpp'
330--- tests/unittests/ut_editor/ut_editor.cpp 2013-12-19 15:55:17 +0000
331+++ tests/unittests/ut_editor/ut_editor.cpp 2014-06-11 13:16:05 +0000
332@@ -69,6 +69,8 @@
333 Key key;
334 if (c.isSpace()) {
335 key.setAction(Key::ActionSpace);
336+ } else if (c == '\r') {
337+ key.setAction(Key::ActionReturn);
338 } else {
339 key.setAction(Key::ActionInsert);
340 key.rLabel() = QString(c);
341@@ -101,12 +103,24 @@
342 << false << "Helol Wordl! " << "Helol Wordl! ";
343 QTest::newRow("auto-correct disabled, multiple punctation")
344 << false << "Helol Wordl!! " << "Helol Wordl!! ";
345- QTest::newRow("auto-correct enabled, digits")
346+ QTest::newRow("auto-correct disabled, digits")
347 << false << "Helol Wordl12 " << "Helol Wordl12 ";
348 // QTest::newRow("auto-correct enabled")
349 // << true << "Helol Wordl! " << "Hello World! ";
350 // QTest::newRow("auto-correct enabled, multiple punctation")
351 // << true << "Helol Wordl!! " << "Hello World!! ";
352+
353+ // Tests for the auto-correct and separator-at-end behavior
354+ // FIXME: In the current testing infra, we cannot really test this properly, as we are using the 'backspace' character
355+ // a lot during auto-correction, which is currently not handled too well here.
356+ QTest::newRow("auto-correct enabled, commit with space, check separators")
357+ << true << "Hel ,Wor ." << "Hello , World . ";
358+ QTest::newRow("auto-correct enabled, commit with separators, check separators")
359+ << true << "Hel.Wor." << "Hello. World. ";
360+ QTest::newRow("auto-correct enabled, check if two spaces are full-stop")
361+ << true << "Hel " << "Hello . ";
362+ //QTest::newRow("auto-correct enabled, check removal of unnecessary whitespaces")
363+ // << true << "Hello. . " << "Hello.. ";
364 }
365
366 Q_SLOT void testAutoCorrect()
367@@ -169,14 +183,24 @@
368 << false << "This is a \"first sentence\". And a second, one! "
369 << "This is a \"first sentence\". And a second, one! " << 2;
370 QTest::newRow("auto-correct enabled, multiple sentences with mixed punctation")
371- << true << "This is a \"first sentence\". And a second, one! "
372+ << true << "This is a \"first sentence\".And a second,one!"
373 << "This is a \"first sentence\". And a second, one! " << 2;
374 QTest::newRow("auto-correct disabled, multiple sentences with dots")
375 << false << "First sentence. Second one. And Third. "
376 << "First sentence. Second one. And Third. " << 3;
377 QTest::newRow("auto-correct enabled, multiple sentences with dots")
378- << true << "First sentence. Second one. And Third. "
379+ << true << "First sentence.Second one.And Third."
380 << "First sentence. Second one. And Third. " << 3;
381+
382+ // Tests for the auto-correct and autocaps separator functionality
383+ // FIXME: In the current testing infra, we cannot really test this properly, as we are using the 'backspace' character
384+ // a lot during auto-correction, which is currently not handled too well here.
385+ QTest::newRow("auto-correct enabled, autocaps after separator auto-correction")
386+ << true << "Hello Wor .This should be autocapsed "
387+ << "Hello World . This should be autocapsed " << 1;
388+ QTest::newRow("auto-correct enabled, autocaps after separator auto-correction #2")
389+ << true << "Hello Wor .This should be autocapsed "
390+ << "Hello World . This should be autocapsed " << 1;
391 }
392
393 Q_SLOT void testAutoCaps()
394@@ -196,6 +220,7 @@
395 initializeWordEngine(word_engine);
396
397 editor.wordEngine()->setWordPredictionEnabled(true);
398+ editor.wordEngine()->setEnabled(true);
399 editor.setAutoCorrectEnabled(enable_auto_correct);
400 editor.setPreeditEnabled(true);
401 editor.setAutoCapsEnabled(true);
402@@ -204,7 +229,7 @@
403
404 QCOMPARE(host.commitStringHistory(), expected_commit_history);
405 Q_UNUSED(expected_auto_caps_activated_count)
406-// QCOMPARE(auto_caps_activated_spy.count(), expected_auto_caps_activated_count);
407+ QCOMPARE(auto_caps_activated_spy.count(), expected_auto_caps_activated_count);
408 }
409 };
410
411
412=== modified file 'tests/unittests/ut_editor/wordengineprobe.cpp'
413--- tests/unittests/ut_editor/wordengineprobe.cpp 2014-04-24 13:18:54 +0000
414+++ tests/unittests/ut_editor/wordengineprobe.cpp 2014-06-11 13:16:05 +0000
415@@ -31,6 +31,38 @@
416
417 #include "wordengineprobe.h"
418
419+// To properly mock language features, we need to realistically determine separators and autoCaps
420+// Since unittests use latin letters, we simply use the same methods as in WesternLanguageFeatures
421+bool MockLanguageFeatures::activateAutoCaps(const QString &preedit) const
422+{
423+ static const QString sentenceBreak = QString::fromUtf8("!.?:\r\n");
424+
425+ if (preedit.isEmpty()) {
426+ return false;
427+ }
428+
429+ if (sentenceBreak.contains(preedit.right(1))) {
430+ return true;
431+ }
432+
433+ return false;
434+}
435+
436+bool MockLanguageFeatures::isSeparator(const QString &text) const
437+{
438+ static const QString separators = QString::fromUtf8(",.!?:;\r\n");
439+
440+ if (text.isEmpty()) {
441+ return false;
442+ }
443+
444+ if (separators.contains(text.right(1))) {
445+ return true;
446+ }
447+
448+ return false;
449+}
450+
451 namespace MaliitKeyboard {
452 namespace Logic {
453
454
455=== modified file 'tests/unittests/ut_editor/wordengineprobe.h'
456--- tests/unittests/ut_editor/wordengineprobe.h 2014-04-24 13:18:54 +0000
457+++ tests/unittests/ut_editor/wordengineprobe.h 2014-06-11 13:16:05 +0000
458@@ -43,10 +43,12 @@
459 explicit MockLanguageFeatures() {}
460 virtual ~MockLanguageFeatures() {}
461
462+ virtual bool isSeparator(const QString &text) const;
463 virtual bool alwaysShowSuggestions() const { return false; }
464- virtual bool autoCapsAvailable() const { return false; }
465- virtual bool activateAutoCaps(const QString &preedit) const { Q_UNUSED(preedit); return false; }
466- virtual QString appendixForReplacedPreedit(const QString &preedit) const { Q_UNUSED(preedit); return ""; }
467+ virtual bool autoCapsAvailable() const { return true; }
468+ virtual bool activateAutoCaps(const QString &preedit) const;
469+ virtual QString appendixForReplacedPreedit(const QString &preedit) const { Q_UNUSED(preedit); return " "; }
470+ virtual QString fullStopSequence() const { return QString("."); }
471 };
472
473 namespace MaliitKeyboard {

Subscribers

People subscribed via source and target branches