GNOME Bugzilla – Bug 341798
More helpful message when there are no search results
Last modified: 2006-05-29 01:55:34 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.
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).
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.
Passing strings from yelp to xsl was more intrusive so I didn't take this path. I'll cook a patch.
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.
Moving to search component.
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.)
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.
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.
Is this OK to commit ?
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.
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.
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.
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.
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() < 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...
Seems to work ok with Beagle. I vote for it to be applied.
Just tested it. Seems to work okay. Feel free to commit, if you have access. Thanks.
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)
Closing.
Thank you!