Merge lp:~pete-woods/hud/tweak-search-parameters into lp:hud/14.04

[Pete Woods]

Tweak search parameters following advice from Jussi

* Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)
  * Yes
 * Did you build your software in a clean sbuild/pbuilder chroot or ppa?
  * Yes
 * Did you build your software in a clean sbuild/pbuilder armhf chroot or ppa?
  * Yes
 * Has your component "TestPlan” been executed successfully on emulator, N4?
  * Yes
 * Has a 5 minute exploratory testing run been executed on N4?
  * Yes
 * If you changed the packaging (debian), did you subscribe a core-dev to this MP?
  * N/A
 * If you changed the UI, did you subscribe the design-reviewers to this MP?
  * No change
 * What components might get impacted by your changes?
  * Unity7
  * Unity8
 * Have you requested review by the teams of these owning components?
  * Yes

Check List:
https://wiki.ubuntu.com/Process/Merges/Checklists/hud

Test Plan:
https://wiki.ubuntu.com/Process/Merges/TestPlan/hud

Silo:
https://launchpad.net/~ci-train-ppa-service/+archive/landing-002/

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:377
http://jenkins.qa.ubuntu.com/job/hud-ci/238/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/hud-trusty-amd64-ci/89
    SUCCESS: http://jenkins.qa.ubuntu.com/job/hud-trusty-armhf-ci/88
    SUCCESS: http://jenkins.qa.ubuntu.com/job/hud-trusty-i386-ci/88

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/hud-ci/238/rebuild

review: Approve (continuous-integration)

[Sebastien Bacher (seb128)]

Thanks for the work, could you explain what issues you are trying to fix with the parameters tweaks? Could you give an example of a query which was considered buggy and what are the differences in the results now?

review: Needs Information

[Pete Woods (pete-woods)]

> Thanks for the work, could you explain what issues you are trying to fix with
> the parameters tweaks? Could you give an example of a query which was
> considered buggy and what are the differences in the results now?

The branch is not ready for merge, but is part of an investigation into the (now) linked bug. I expect there will additionally be changes made to libcolumbus. I will link the relevant columbus branch to this bug, also.

The test application is essentially GIMP. If you search for, for example, "crop to selection" (assuming you have something selected), you will see that you don't exactly get the results you'd expect. The same applies to the other cases I have been testing "fit canvas to layers" and "fit canvas to selection".

[Sebastien Bacher (seb128)]

the new parameters have regressions. One example, open gedit in french and type "pref" -> empty results, it used to list "préférence", adding a char "prefe" and it's still empty, you need to type "prefer" to get a result.

[Pete Woods (pete-woods)]

> the new parameters have regressions. One example, open gedit in french and
> type "pref" -> empty results, it used to list "préférence", adding a char
> "prefe" and it's still empty, you need to type "prefer" to get a result.

Hmm, that's annoying, the unit tests (http://bazaar.launchpad.net/~pete-woods/hud/tweak-search-parameters/view/head:/tests/unit/service/TestItemStore.cpp#L181) are clearly not strong enough for this regression!

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:378
http://jenkins.qa.ubuntu.com/job/hud-ci/239/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-amd64-ci/90/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-armhf-ci/89/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-i386-ci/89/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/hud-ci/239/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:379
http://jenkins.qa.ubuntu.com/job/hud-ci/240/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-amd64-ci/91/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-armhf-ci/90/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-i386-ci/90/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/hud-ci/240/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:380
http://jenkins.qa.ubuntu.com/job/hud-ci/241/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-amd64-ci/92/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-armhf-ci/91/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-i386-ci/91/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/hud-ci/241/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:381
http://jenkins.qa.ubuntu.com/job/hud-ci/242/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-amd64-ci/93/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-armhf-ci/92/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-i386-ci/92/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/hud-ci/242/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:384
http://jenkins.qa.ubuntu.com/job/hud-ci/243/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-amd64-ci/94/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-armhf-ci/93/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-i386-ci/93/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/hud-ci/243/rebuild

review: Needs Fixing (continuous-integration)

[lp:~pete-woods/hud/tweak-search-parameters]

375. By Pete Woods on 2014-03-04

Change GTK documentation so that it builds with the new glib version Fixes: 1287580

376. By PS Jenkins bot on 2014-03-04

Releasing 13.10.1+14.04.20140304-0ubuntu1

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:386
http://jenkins.qa.ubuntu.com/job/hud-ci/254/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/hud-trusty-amd64-ci/105/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/hud-trusty-armhf-ci/104
    SUCCESS: http://jenkins.qa.ubuntu.com/job/hud-trusty-i386-ci/104

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/hud-ci/254/rebuild

review: Needs Fixing (continuous-integration)

[Antti Kaijanmäki (kaijanmaki)]

LGTM. Waiting for silo.

[Antti Kaijanmäki (kaijanmaki)]

 * Are any changes against your component pending/needed to land the MP under review in a functional state and are those called out explicitly by the submitter?
Yes.

 * Did you do exploratory testing related to the component you own with the MP changeset included?
Yes.

 * Has the submitter requested review by all the relevant teams/reviewres?
Yes.

 * If you are the reviewer owning the component the MP is against, have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?
Yes.

review: Approve

[Charles Kerr (charlesk)]

I notice this approved and waiting for silo even though the only changes since seb's "préférence" regression are docs/packaging changes. Are we releasing with that regression? Is it fixed upstream in libcolumbus?

review: Needs Information

[Pete Woods (pete-woods)]

> I notice this approved and waiting for silo even though the only changes since
> seb's "préférence" regression are docs/packaging changes. Are we releasing
> with that regression? Is it fixed upstream in libcolumbus?

Don't worry! The history just looks funny because I rebased.

I specifically strengthen the test for what Seb was talking about in revision 384. I also installed the French language pack and checked the search manually.

[Pete Woods (pete-woods)]

http://i.imgur.com/n3K3ZDo.png

[Sebastien Bacher (seb128)]

> http://i.imgur.com/n3K3ZDo.png

;-)

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-02-10 12:13:59 +0000
+++ CMakeLists.txt	2014-03-14 14:07:40 +0000
@@ -163,9 +163,9 @@
 		check
 		${CMAKE_CTEST_COMMAND} --force-new-ctest-process --output-on-failure
 	)
+	add_subdirectory(tests)
 endif()
 
-add_subdirectory(tests)
 
 add_subdirectory(tools-vala)
 if(${ENABLE_DOCUMENTATION})

=== modified file 'data/com.canonical.indicator.appmenu.hud.search.gschema.xml'
--- data/com.canonical.indicator.appmenu.hud.search.gschema.xml	2012-04-02 15:57:16 +0000
+++ data/com.canonical.indicator.appmenu.hud.search.gschema.xml	2014-03-14 14:07:40 +0000
@@ -15,7 +15,7 @@
     </key>
 
     <key type='u' name='add-penalty'>
-      <default>10</default>
+      <default>100</default>
       <summary>Penalty for extra characters added to the search</summary>
       <description>
         The penalty for each extra character in the search string that does not appear in the text of a menu
@@ -27,7 +27,7 @@
     </key>
 
     <key type='u' name='drop-penalty'>
-      <default>10</default>
+      <default>100</default>
       <summary>Penalty applied if a character is dropped</summary>
       <description>
         The penalty for each character dropped from the search string, as compared with the text of a menu item.
@@ -39,7 +39,7 @@
     </key>
 
     <key type='u' name='end-drop-penalty'>
-      <default>1</default>
+      <default>20</default>
       <summary>Penalty applied if a character is dropped from the end</summary>
       <description>
         The penalty for each missing character at the end of a search term.
@@ -50,7 +50,7 @@
     </key>
 
     <key type='u' name='swap-penalty'>
-      <default>15</default>
+      <default>150</default>
       <summary>Penalty applied when the characters are not the same</summary>
       <description>
         The penalty for each substituted character in the search term.

=== modified file 'debian/control'
--- debian/control	2014-02-10 12:13:59 +0000
+++ debian/control	2014-03-14 14:07:40 +0000
@@ -9,7 +9,7 @@
                google-mock (>= 1.6.0+svn437),
                gtk-doc-tools,
                intltool,
-               libcolumbus1-dev,
+               libcolumbus1-dev (>= 1.1.0),
                libdbusmenu-glib-dev (>= 0.5.90),
                libdbusmenu-gtk3-dev (>= 0.5.90),
                libdbusmenu-jsonloader-dev (>= 0.5.90),

=== modified file 'service/HardCodedSearchSettings.h'
--- service/HardCodedSearchSettings.h	2014-02-17 09:58:27 +0000
+++ service/HardCodedSearchSettings.h	2014-03-14 14:07:40 +0000
@@ -51,9 +51,9 @@
 
 	uint m_dropPenalty = 100;
 
-	uint m_endDropPenalty = 1;
+	uint m_endDropPenalty = 20;
 
-	uint m_swapPenalty = 5;
+	uint m_swapPenalty = 150;
 };
 
 }

=== modified file 'service/ItemStore.cpp'
--- service/ItemStore.cpp	2014-03-14 14:07:39 +0000
+++ service/ItemStore.cpp	2014-03-14 14:07:40 +0000
@@ -29,6 +29,7 @@
 using namespace Columbus;
 
 static const QRegularExpression SINGLE_AMPERSAND("(?<![&])[&](?![&])");
+static const QRegularExpression BAD_CHARACTERS("\\.\\.\\.|…");
 static const QRegularExpression WHITESPACE("\\s+");
 static const QRegularExpression WHITESPACE_OR_SEMICOLON("[;\\s+]");
 
@@ -39,8 +40,9 @@
 	ErrorValues &errorValues(m_matcher.getErrorValues());
 	errorValues.addStandardErrors();
 
-	connect(m_settings.data(), SIGNAL(changed()), this,
-					SLOT(settingChanged()));
+	m_matcher.getIndexWeights().setWeight(Word("context"), 0.5);
+
+	connect(m_settings.data(), SIGNAL(changed()), this, SLOT(settingChanged()));
 	settingChanged();
 }
 
@@ -72,7 +74,9 @@
 			continue;
 		}
 
-		QStringList text(convertActionText(action).split(WHITESPACE));
+		QStringList text(
+				convertActionText(action).remove(BAD_CHARACTERS).split(
+						WHITESPACE));
 
 		bool isParameterized(action->property("isParameterized").toBool());
 
@@ -89,7 +93,7 @@
 
 			WordList command;
 			for (const QString &word : text) {
-				command.addWord(Word(word.toStdString()));
+				command.addWord(Word(word.toUtf8().constData()));
 			}
 			document.addText(Word("command"), command);
 
@@ -102,7 +106,7 @@
 				context = stack;
 			}
 			for (const QString &word : context) {
-				wordList.addWord(Word(word.toStdString()));
+				wordList.addWord(Word(word.toUtf8().constData()));
 			}
 			document.addText(Word("context"), wordList);
 
@@ -177,21 +181,28 @@
 		}
 
 	} else {
+		QString cleanQuery(query);
+		cleanQuery.remove(BAD_CHARACTERS);
+
 		WordList queryList;
-		for (const QString &word : query.split(WHITESPACE)) {
-			queryList.addWord(word.toStdString());
+		for (const QString &word : cleanQuery.split(WHITESPACE)) {
+			queryList.addWord(Word(word.toUtf8().constData()));
 		}
 
-		MatchResults matchResults(m_matcher.match(queryList));
-
-		int queryLength(query.length());
-
-		size_t maxResults = std::min(matchResults.size(), size_t(20));
-
-		for (size_t i(0); i < maxResults; ++i) {
-			DocumentID id(matchResults.getDocumentID(i));
-			double relevancy(matchResults.getRelevancy(i));
-			addResult(id, stringMatcher, queryLength, relevancy, results);
+		try {
+			MatchResults matchResults(
+					m_matcher.onlineMatch(queryList, Word("command")));
+
+			int queryLength(query.length());
+
+			size_t maxResults = std::min(matchResults.size(), size_t(20));
+
+			for (size_t i(0); i < maxResults; ++i) {
+				DocumentID id(matchResults.getDocumentID(i));
+				double relevancy(matchResults.getRelevancy(i));
+				addResult(id, stringMatcher, queryLength, relevancy, results);
+			}
+		} catch (std::invalid_argument &e) {
 		}
 	}
 

=== modified file 'service/QGSettingsSearchSettings.cpp'
--- service/QGSettingsSearchSettings.cpp	2014-02-15 10:23:45 +0000
+++ service/QGSettingsSearchSettings.cpp	2014-03-14 14:07:40 +0000
@@ -34,11 +34,11 @@
 }
 
 uint QGSettingsSearchSettings::addPenalty() const {
-	return m_settings.get("addPenalty").toUInt() * 10;
+	return m_settings.get("addPenalty").toUInt();
 }
 
 uint QGSettingsSearchSettings::dropPenalty() const {
-	return m_settings.get("dropPenalty").toUInt() * 10;
+	return m_settings.get("dropPenalty").toUInt();
 }
 
 uint QGSettingsSearchSettings::endDropPenalty() const {

=== modified file 'tests/integration/TestHud.cpp'
--- tests/integration/TestHud.cpp	2014-01-24 14:25:22 +0000
+++ tests/integration/TestHud.cpp	2014-03-14 14:07:40 +0000
@@ -187,8 +187,8 @@
 		countChangedSpy.wait();
 	}
 	ASSERT_EQ(3, results.rowCount());
-	EXPECT_EQ(ResultPair("swift sad", "piece hook"), result(results, 0));
-	EXPECT_EQ(ResultPair("stray slash", "piece hook"), result(results, 1));
+	EXPECT_EQ(ResultPair("stray slash", "piece hook"), result(results, 0));
+	EXPECT_EQ(ResultPair("swift sad", "piece hook"), result(results, 1));
 	EXPECT_EQ(ResultPair("bowl", "link"), result(results, 2));
 }
 

=== modified file 'tests/unit/service/TestItemStore.cpp'
--- tests/unit/service/TestItemStore.cpp	2014-03-14 14:07:39 +0000
+++ tests/unit/service/TestItemStore.cpp	2014-03-14 14:07:40 +0000
@@ -222,8 +222,11 @@
 	QMenu root;
 
 	QMenu file("File");
+	file.addAction("Banana");
+	file.addAction("Save All");
+	file.addAction("Save");
 	file.addAction("Save As...");
-	file.addAction("Save");
+	file.addAction("Apple");
 	root.addMenu(&file);
 
 	store->indexMenu(&root);