After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 341798 - More helpful message when there are no search results
More helpful message when there are no search results
Status: RESOLVED FIXED
Product: yelp
Classification: Applications
Component: Search
2.14.x
Other Linux
: Normal enhancement
: ---
Assigned To: Yelp maintainers
Yelp maintainers
Depends on:
Blocks: 341434 341689
 
 
Reported: 2006-05-15 04:41 UTC by Matthew Paul Thomas (mpt)
Modified: 2006-05-29 01:55 UTC
See Also:
GNOME target: ---
GNOME version: 2.13/2.14


Attachments
Helpful message when there are no search results (3.22 KB, patch)
2006-05-26 11:57 UTC, Frederic Peters
none Details | Review
Helpful messages & translatable strings (3.14 KB, patch)
2006-05-26 14:38 UTC, Frederic Peters
none Details | Review
Helpful messages & translatable strings (3.13 KB, patch)
2006-05-27 09:26 UTC, Frederic Peters
none Details | Review
Helpful message & translatable strings (3.64 KB, patch)
2006-05-28 15:05 UTC, Frederic Peters
committed Details | Review

Description Matthew Paul Thomas (mpt) 2006-05-15 04:41:02 UTC
Yelp 2.14.1, Ubuntu Dapper

Searching for a term that isn't included anywhere in the help (such as "fqwhgads") returns only the line "0 result(s) returned.".

This message could be much more helpful. Something like:

    No results for “fqhwgads”

    Try using different words to describe the problem you're having or the
    topic you want help with.
Comment 1 Frederic Peters 2006-05-26 11:57:01 UTC
Created attachment 66272 [details] [review]
Helpful message when there are no search results

This implements exactly what mpt requested.

Note that exposing better messages in xsl shows it is necessary to have a translation mechanism for xsl files (also noted in #341689).
Comment 2 Don Scorgie 2006-05-26 13:41:10 UTC
Thanks for the patch!  I haven't been looking at search recently (still battling with info pages - but that's a different story).

I was thinking about this issue though.  I was wondering if, instead of passing in the number of results and creating the string in the xslt, it would be better to construct the string within the code and pass it in.  I haven't really played with this yet, but something like:

params[params_i++] = "yelp.searchterms";
if (number_of_results > 0 && number_of_results < 10)
 params[parames_i++] = g_strdup_printf ("Search results for %s", search_terms);

else if (number_of_results == 0)
 params[params_i++] = g_strdup_printf ("No search results found for %s\nTry Altering your search terms blah blah blah", search_terms);

else /* Too many results.  Just showing the first few.  Another bug.  Forget which one*/
 params[params_i++] = g_strdup_printf ("Search results for %s.\nThere are too many results for yelps puny mind.  Try narrowing your query, possibly by adding extra words", search_terms);

With the appropriate gettext calls etc. (Also, some formatting tags maybe?)  This would remove the need to gettextise the xslt stuff.  It may require some ammending to the xslt stylesheet.

Also, just a comment on the code:
+    for (j=0; searchterms[j]; j++) {
+    	if (searchterms[j] == '"') {
+	    searchterms[j] = ' ';
+	}
+    }

It'd be a little cleaner to replace the entire loop with one call to g_str_escape ().

I'll have a proper look when I get home though.
Comment 3 Frederic Peters 2006-05-26 13:53:08 UTC
Passing strings from yelp to xsl was more intrusive so I didn't take this path.

I'll cook a patch.
Comment 4 Frederic Peters 2006-05-26 14:38:19 UTC
Created attachment 66286 [details] [review]
Helpful messages & translatable strings

Combo patch, fixing bug 341798 (this one), bug 341434 ("don't return hundreds of results") and bug 341689 (make string translatable).

It uses another logic than the first patch, not using xsl:param, since they don't work well with quotes and translated strings sure will have some in bad places.  So it simply adds xml text nodes to the root.

It displays the helpful "you should refine your search" if there are more than 20 items; and it cuts down the list to only display those.
Comment 5 Don Scorgie 2006-05-26 18:04:30 UTC
Moving to search component.
Comment 6 Matthew Paul Thomas (mpt) 2006-05-27 04:46:22 UTC
Thanks, Frederic! A couple of suggestions:

+    	title = g_strdup_printf( _("No search result for \"%s\""), priv->search_terms);

"No search result" should be "No results", because fewer words with the same effect is better, and "result" would incorrectly suggest that successful searches have exactly one result.

+        if (number_of_results > 20) {
+            text = g_strdup(_("There are too many results for yelps puny mind. "
+	    		      "Try narrowing your query, possibly by adding "
+			      "extra words."));

The idea of limiting results to 20 is because people won't look past the first page. It's not because achieving 20 or fewer results is something users should be trying to do! So if there are more than 20 results, don't show any error message, just show the top 20. (If those 20 aren't the most relevant pages for any given search, that should be fixed in the ranking algorithm -- e.g. bug 341797 and bug 341800 -- and/or the docs themselves.)
Comment 7 Frederic Peters 2006-05-27 09:26:18 UTC
Created attachment 66325 [details] [review]
Helpful messages & translatable strings

Updated with strings matching last comment.

English is not my native tongue so it needs review:

  There are too many results matching your query.
  Try narrowing your query, possibly by adding extra words.
Comment 8 Matthew Paul Thomas (mpt) 2006-05-28 09:32:31 UTC
Sorry, I'll be less wordy: Please do not show any error message when there are more than 20 results. Just show the top 20 results, and be happy. (I guess that means, delete the four lines starting with "if (number_of_results > 20) {".)

Thanks for fixing the grammar of the other message.
Comment 9 Frederic Peters 2006-05-28 12:50:43 UTC
Is this OK to commit ?
Comment 10 Brent Smith (smitten) 2006-05-28 14:50:39 UTC
Looks good to me, please remove the period at the end of _("Search results for \"%s\".")

Don, can you please take a look and see what you think?  I'm not sure if limiting the results to 20 hits is a good thing, since I don't know how well the search priority works right now.

Also it might make sense to number our results.
Comment 11 Frederic Peters 2006-05-28 15:05:08 UTC
Created attachment 66376 [details] [review]
Helpful message & translatable strings

Updated to match all comments and added a ChangeLog entry; waiting for Don approval to commit.
Comment 12 Don Scorgie 2006-05-28 17:26:08 UTC
Sorry, been a little pe-occupied of late and haven't had a chance to try out the patch yet.  A few (very minor) points though:

+    xmlXPathContextPtr results_xpath_ctx = NULL;
+    xmlXPathObjectPtr results_xpath = NULL;

These both need to be freed at some point.

I'm somewhat nervous about grabbing the number_of_results through counting the node number.  It's probably a load more work, but it'd be better to keep a track in YelpSearchPagerPriv somewhere of the number of results generated and use that.  I have no idea whether that's doable, but it just seems less likely to go wrong (if, for some reason the number of nodes changes.  Yes, I do have something specific in mind).

smitten: The limiting to 20 hits is (I think) okay.  If nothing else, it'll force me / someone to look at the search priority and make it better (it is again on my todo list).  In the mean time, limiting to 20 hits will capture most of the relevant results.

Fredric: Have you tried this with Beagle enabled?  Reading it through, it shouldn't have any impact, but...

Other than that, it looks okay.  I'll try it out soon.  Also marking the various other bugs as dependants.
Comment 13 Frederic Peters 2006-05-28 17:40:50 UTC
They are already freeed (but not set to NULL afterwards), see: 
  xmlXPathFreeObject(results_xpath);
  xmlXPathFreeContext(results_xpath_ctx);

It is possible to add number_of_results to YelpSearchPagerPriv but this will be much more intrusive since all the process_*_result will have to be edited (I was not terribly happy about using XPath to get number_of_results but this is much cleaner).

And I didn't test with Beagle.
Comment 14 Brent Smith (smitten) 2006-05-28 17:57:36 UTC
I've tried it out, it applies to head fine and seems to work.

One more thing:

<xsl:for-each select="result[@uri != '' and position() &lt; 20]">

not sure if the position() function is 0-based or 1-based so I don't know if that returns 19 or 20 results...
Comment 15 Brent Smith (smitten) 2006-05-28 18:01:09 UTC
Seems to work ok with Beagle.  I vote for it to be applied.
Comment 16 Don Scorgie 2006-05-28 18:12:10 UTC
Just tested it.  Seems to work okay.  Feel free to commit, if you have access.  Thanks.
Comment 17 Frederic Peters 2006-05-28 18:34:48 UTC
Done; thanks.  I also checked position() and it is 1-based, so I changed the comparision to get 20 results listed.

2006-06-28  Frederic Peters <fpeters@0d.be>

        * src/yelp-search-pager.c:
        * stylesheets/search2html.xsl:
        more helpful message when there are no search results (fixes #341798)
        search results message can be translated (fixes #341689)
        limit search results to 20 items (fixes #341434)

(I don't have editbugs priviledges, so I can't close it)
Comment 18 Don Scorgie 2006-05-28 18:37:48 UTC
Closing.
Comment 19 Matthew Paul Thomas (mpt) 2006-05-29 01:55:34 UTC
Thank you!