ubuntu-keyboard

Merge lp:~michael-sheldon/ubuntu-keyboard/fix-1579083 into lp:ubuntu-keyboard

  1. fix-1579083
  2. Merge into trunk
Proposed by Michael Sheldon
Status: Merged
Approved by: Bill Filler
Approved revision: 481
Merged at revision: 507
Proposed branch: lp:~michael-sheldon/ubuntu-keyboard/fix-1579083
Merge into: lp:ubuntu-keyboard
Diff against target: 153 lines (+43/-6)
5 files modified
plugins/chewing/src/chewinglanguagefeatures.cpp (+1/-1)
plugins/pinyin/src/chineselanguagefeatures.cpp (+1/-1)
plugins/westernsupport/westernlanguagefeatures.cpp (+1/-1)
src/view/abstracttexteditor.cpp (+29/-2)
tests/unittests/ut_editor/ut_editor.cpp (+11/-1)
To merge this branch: bzr merge lp:~michael-sheldon/ubuntu-keyboard/fix-1579083
Reviewer Review Type Date Requested Status
system-apps-ci-bot continuous-integration Needs Fixing
Ubuntu Phablet Team Pending
Review via email: mp+302925@code.launchpad.net

Commit message

Improve double space punctuation insertion behaviour

Description of the change

Improve double space punctuation insertion behaviour

To post a comment you must log in.
Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

FAILED: Continuous integration, rev:478
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-keyboard-ci/30/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-apps/job/build/1189/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/1189
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=vivid+overlay/1069
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=xenial+overlay/1069
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=yakkety/1069
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1057
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1057/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1057
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1057/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1057/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1057
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1057/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1057
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1057/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1057/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1057
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1057/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1057
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1057/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/1057/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-keyboard-ci/30/rebuild

review: Needs Fixing (continuous-integration)
lp:~michael-sheldon/ubuntu-keyboard/fix-1579083 updated
479. By Michael Sheldon

Remove automatically inserted full-stop if the user continues to insert space characters after a double space

480. By Michael Sheldon

Add unit tests for improved punctuation behaviour

Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

FAILED: Continuous integration, rev:480
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-keyboard-ci/33/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-apps/job/build/1224/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/1224
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=vivid+overlay/1098
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=xenial+overlay/1098
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=yakkety/1098
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1085
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1085/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1085
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1085/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1085/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1085
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1085/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1085
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1085/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1085/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1085
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1085/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1085
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1085/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/1085/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-keyboard-ci/33/rebuild

review: Needs Fixing (continuous-integration)
lp:~michael-sheldon/ubuntu-keyboard/fix-1579083 updated
481. By Michael Sheldon

Add comment on usage of triple space detection

Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

FAILED: Continuous integration, rev:481
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-keyboard-ci/36/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-apps/job/build/1241/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/1241
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=vivid+overlay/1114
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=xenial+overlay/1114
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=yakkety/1114
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1101
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1101/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1101
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1101/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1101/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1101
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1101/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1101
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1101/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1101/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1101
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1101/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1101
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1101/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/1101/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-keyboard-ci/36/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/chewing/src/chewinglanguagefeatures.cpp'
2--- plugins/chewing/src/chewinglanguagefeatures.cpp 2016-04-28 12:15:46 +0000
3+++ plugins/chewing/src/chewinglanguagefeatures.cpp 2016-08-18 13:50:50 +0000
4@@ -55,7 +55,7 @@
5
6 bool ChewingLanguageFeatures::isSeparator(const QString &text) const
7 {
8- static const QString separators = QString::fromUtf8("。、!?:\r\n");
9+ static const QString separators = QString::fromUtf8("。、!?:…\r\n");
10
11 if (text.isEmpty()) {
12 return false;
13
14=== modified file 'plugins/pinyin/src/chineselanguagefeatures.cpp'
15--- plugins/pinyin/src/chineselanguagefeatures.cpp 2015-02-09 21:14:24 +0000
16+++ plugins/pinyin/src/chineselanguagefeatures.cpp 2016-08-18 13:50:50 +0000
17@@ -56,7 +56,7 @@
18
19 bool ChineseLanguageFeatures::isSeparator(const QString &text) const
20 {
21- static const QString separators = QString::fromUtf8("。、,!?:;.\r\n");
22+ static const QString separators = QString::fromUtf8("。、,!?:;.…\r\n");
23
24 if (text.isEmpty()) {
25 return false;
26
27=== modified file 'plugins/westernsupport/westernlanguagefeatures.cpp'
28--- plugins/westernsupport/westernlanguagefeatures.cpp 2016-06-09 15:12:25 +0000
29+++ plugins/westernsupport/westernlanguagefeatures.cpp 2016-08-18 13:50:50 +0000
30@@ -77,7 +77,7 @@
31
32 bool WesternLanguageFeatures::isSeparator(const QString &text) const
33 {
34- static const QString separators = QString::fromUtf8(",.!?:;\r\n");
35+ static const QString separators = QString::fromUtf8(",.!?:;…\r\n");
36
37 if (text.isEmpty()) {
38 return false;
39
40=== modified file 'src/view/abstracttexteditor.cpp'
41--- src/view/abstracttexteditor.cpp 2016-06-15 12:44:24 +0000
42+++ src/view/abstracttexteditor.cpp 2016-08-18 13:50:50 +0000
43@@ -205,6 +205,7 @@
44 int ignore_next_cursor_position;
45 QString ignore_next_surrounding_text;
46 bool look_for_a_double_space;
47+ bool look_for_a_triple_space;
48 bool double_space_full_stop_enabled;
49 bool editing_middle_of_text;
50 QString appendix_for_previous_preedit;
51@@ -236,6 +237,7 @@
52 , ignore_next_cursor_position(-1)
53 , ignore_next_surrounding_text()
54 , look_for_a_double_space(false)
55+ , look_for_a_triple_space(false)
56 , double_space_full_stop_enabled(false)
57 , editing_middle_of_text(false)
58 , appendix_for_previous_preedit()
59@@ -356,6 +358,7 @@
60 QString keyText = QString("");
61 Qt::Key event_key = Qt::Key_unknown;
62 bool look_for_a_double_space = d->look_for_a_double_space;
63+ bool look_for_a_triple_space = d->look_for_a_triple_space;
64 bool email_detected = false;
65
66 // Detect if the user is entering an email address and avoid spacing, autocaps and autocomplete changes
67@@ -368,8 +371,11 @@
68 email_detected = true;
69 }
70
71+ // we reset the flags here so that we won't have to add boilerplate code later
72+ if (d->look_for_a_triple_space) {
73+ d->look_for_a_triple_space = false;
74+ }
75 if (look_for_a_double_space) {
76- // we reset the flag here so that we won't have to add boilerplate code later
77 d->look_for_a_double_space = false;
78 }
79
80@@ -464,6 +470,7 @@
81 case Key::ActionSpace: {
82 QString space = " ";
83 QString textOnLeft = d->text->surroundingLeft() + d->text->preedit();
84+ QString textOnLeftTrimmed = textOnLeft.trimmed();
85 QStringList textOnRightList = d->text->surroundingRight().split("\n");
86 QString textOnRight;
87 if (!textOnRightList.isEmpty()) {
88@@ -485,6 +492,14 @@
89 }
90 }
91
92+ // Delete automatically inserted full stop if the user continues
93+ // pressing space after a double space.
94+ if (look_for_a_triple_space) {
95+ singleBackspace();
96+ singleBackspace();
97+ d->text->appendToPreedit(" ");
98+ }
99+
100 if (replace_preedit) {
101 if (!textOnRight.isEmpty() && d->editing_middle_of_text) {
102 // Don't insert a space if we are correcting a word in the middle of a sentence
103@@ -512,7 +527,18 @@
104 d->previous_preedit_position -= 1;
105 }
106 }
107- else if (look_for_a_double_space && not stopSequence.isEmpty() && textOnLeft.at(textOnLeft.count() - 1).isSpace()) {
108+ // Only insert a full stop after double space if there isn't already
109+ // a separator, and there isn't a separator immediately prior to a ')'
110+ else if (look_for_a_double_space
111+ && not stopSequence.isEmpty()
112+ && textOnLeft.count() >= 2
113+ && textOnLeft.at(textOnLeft.count() - 1).isSpace()
114+ && !textOnLeft.at(textOnLeft.count() - 2).isSpace()
115+ && textOnLeftTrimmed.count() > 0
116+ && !d->word_engine->languageFeature()->isSeparator(textOnLeftTrimmed.at(textOnLeftTrimmed.count() - 1))
117+ && !(textOnLeftTrimmed.endsWith(")")
118+ && textOnLeftTrimmed.count() > 1
119+ && d->word_engine->languageFeature()->isSeparator(textOnLeftTrimmed.at(textOnLeftTrimmed.count() - 2)))) {
120 removeTrailingWhitespaces();
121 if (!d->word_engine->languageFeature()->commitOnSpace()) {
122 // Commit when inserting a fullstop if we don't insert on spaces
123@@ -529,6 +555,7 @@
124 textOnLeft = d->text->surroundingLeft() + d->text->preedit();
125 auto_caps_activated = d->word_engine->languageFeature()->activateAutoCaps(textOnLeft);
126 full_stop_inserted = true;
127+ d->look_for_a_triple_space = true;
128 }
129
130 d->text->appendToPreedit(space);
131
132=== modified file 'tests/unittests/ut_editor/ut_editor.cpp'
133--- tests/unittests/ut_editor/ut_editor.cpp 2014-12-04 17:44:17 +0000
134+++ tests/unittests/ut_editor/ut_editor.cpp 2016-08-18 13:50:50 +0000
135@@ -118,7 +118,17 @@
136 QTest::newRow("auto-correct enabled, commit with separators, check separators")
137 << true << "Hel. Wor." << "Hello. World.";
138 QTest::newRow("auto-correct enabled, check if two spaces are full-stop")
139- << true << "Hel " << "Hello . ";
140+ << true << "Hel " << "Hello . ";
141+ QTest::newRow("auto-correct enabled, check two spaces don't insert full-stop after .")
142+ << true << "Hello. " << "Hello. ";
143+ QTest::newRow("auto-correct enabled, check two spaces don't insert full-stop after !")
144+ << true << "Hello! " << "Hello! ";
145+ QTest::newRow("auto-correct enabled, check two spaces don't insert full-stop after ?")
146+ << true << "Hello? " << "Hello? ";
147+ QTest::newRow("auto-correct enabled, check two spaces don't insert full-stop after ...")
148+ << true << "Hello... " << "Hello... ";
149+ QTest::newRow("auto-correct enabled, check two spaces does insert full-stop after )")
150+ << true << "(Hello) " << "(Hello) . ";
151 //QTest::newRow("auto-correct enabled, check removal of unnecessary whitespaces")
152 // << true << "Hello. . " << "Hello.. ";
153 }

Subscribers

People subscribed via source and target branches