Merge lp:~sil2100/ubuntu-keyboard/enhance_word_completion into lp:ubuntu-keyboard
- enhance_word_completion
- Merge into trunk
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 |
Related bugs: |
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.
PS Jenkins bot (ps-jenkins) wrote : | # |
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
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:154
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Ł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
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:155
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 156. By Łukasz Zemczak
-
Add some additional comments
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:156
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 157. By Łukasz Zemczak
-
Add the possibility to commit strings using separators (like dots and commas). Fix pinyin input to consider separators
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:157
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 158. By Łukasz Zemczak
-
Merge trunk
- 159. By Łukasz Zemczak
-
Fix the tests, still working on fixing all edge-cases
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:159
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 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
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:161
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:162
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
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.autoCapsT
Ł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
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:163
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
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
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"
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/
- 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
Ł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 :)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:165
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
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.
Michael Sheldon (michael-sheldon) : | # |
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: )"
Łukasz Zemczak (sil2100) wrote : | # |
Ah! Smileys ;) Forgot about those! Let me make a quick fix soon.
Ł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
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:166
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 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
Ł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.
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:168
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
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:/
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
Preview Diff
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 { |
PASSED: Continuous integration, rev:152 jenkins. qa.ubuntu. com/job/ ubuntu- keyboard- ci/349/ jenkins. qa.ubuntu. com/job/ ubuntu- keyboard- trusty- amd64-ci/ 178 jenkins. qa.ubuntu. com/job/ ubuntu- keyboard- trusty- armhf-ci/ 178 jenkins. qa.ubuntu. com/job/ ubuntu- keyboard- trusty- armhf-ci/ 178/artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ ubuntu- keyboard- trusty- i386-ci/ 177
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/ubuntu- keyboard- ci/349/ rebuild
http://