From ec18eb40eaf8145af3004f65ad98538a18f00ade Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Fri, 25 Feb 2022 15:10:36 +0100 Subject: [PATCH 1/6] Readd a `SearchRenderer` constructor without `Library` argument. Adding the library argument breaks the API. It is better to add another constructor to not have to create another major version. --- include/search_renderer.h | 18 ++++++++++++++++-- src/search_renderer.cpp | 5 +++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/include/search_renderer.h b/include/search_renderer.h index 673e31ebe..b16567ee1 100644 --- a/include/search_renderer.h +++ b/include/search_renderer.h @@ -49,10 +49,24 @@ class SearchRenderer /** * Construct a SearchRenderer from a SearchResultSet. * + * The constructed version of the SearchRendered will not introduce + * the book name for each result. It is better to use the another constructor + * with a Library pointer to have a better html page. + * * @param srs The `SearchResultSet` to render. * @param mapper The `NameMapper` to use to do the rendering. - * @param library The `Library` to use to look up book details for search - * results + * @param start The start offset used for the srs. + * @param estimatedResultCount The estimatedResultCount of the whole search + */ + SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, + unsigned int start, unsigned int estimatedResultCount); + + /** + * Construct a SearchRenderer from a SearchResultSet. + * + * @param srs The `SearchResultSet` to render. + * @param mapper The `NameMapper` to use to do the rendering. + * @param library The `Library` to use to look up book details for search results. * @param start The start offset used for the srs. * @param estimatedResultCount The estimatedResultCount of the whole search */ diff --git a/src/search_renderer.cpp b/src/search_renderer.cpp index 4b6b8e90b..08f09a1ed 100644 --- a/src/search_renderer.cpp +++ b/src/search_renderer.cpp @@ -47,6 +47,11 @@ SearchRenderer::SearchRenderer(Searcher* searcher, NameMapper* mapper) resultStart(searcher->getResultStart()) {} +SearchRenderer::SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, + unsigned int start, unsigned int estimatedResultCount) + : SearchRenderer(srs, mapper, nullptr, start, estimatedResultCount) +{} + SearchRenderer::SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, Library* library, unsigned int start, unsigned int estimatedResultCount) : m_srs(srs), From 921671eb4d1135b67cad872e42b4a5b7b9857166 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Fri, 25 Feb 2022 15:13:55 +0100 Subject: [PATCH 2/6] Do not use ostringstream to convert the uuid into string. `zim::Uuid` already have a string convertion operator. Let's use it. --- src/search_renderer.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/search_renderer.cpp b/src/search_renderer.cpp index 08f09a1ed..8d089b56f 100644 --- a/src/search_renderer.cpp +++ b/src/search_renderer.cpp @@ -95,12 +95,11 @@ std::string SearchRenderer::getHtml() result.set("title", it.getTitle()); result.set("url", it.getPath()); result.set("snippet", it.getSnippet()); - std::ostringstream s; - s << it.getZimId(); - result.set("resultContentId", mp_nameMapper->getNameForId(s.str())); + std::string zim_id(it.getZimId()); + result.set("resultContentId", mp_nameMapper->getNameForId(zim_id)); std::shared_ptr archive; try { - result.set("bookTitle", mp_library->getBookById(s.str()).getTitle()); + result.set("bookTitle", mp_library->getBookById(zim_id).getTitle()); } catch (const std::out_of_range& e) {} if (it.getWordCount() >= 0) { From d9124ed40bfa601a2f44c9ffc542940bcb91e027 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Fri, 25 Feb 2022 15:15:04 +0100 Subject: [PATCH 3/6] Set the book title only if we have a library. --- src/search_renderer.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/search_renderer.cpp b/src/search_renderer.cpp index 8d089b56f..2b55dd902 100644 --- a/src/search_renderer.cpp +++ b/src/search_renderer.cpp @@ -98,9 +98,11 @@ std::string SearchRenderer::getHtml() std::string zim_id(it.getZimId()); result.set("resultContentId", mp_nameMapper->getNameForId(zim_id)); std::shared_ptr archive; - try { + if (!mp_library) { + result.set("bookTitle", kainjow::mustache::data(false)); + } else { result.set("bookTitle", mp_library->getBookById(zim_id).getTitle()); - } catch (const std::out_of_range& e) {} + } if (it.getWordCount() >= 0) { result.set("wordCount", kiwix::beautifyInteger(it.getWordCount())); From 609bc24cbe270026e89e36f497b321406a601cb4 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Fri, 25 Feb 2022 15:15:41 +0100 Subject: [PATCH 4/6] Small cleanups. - Remove unused `archive` - Replace tab by spaces --- src/search_renderer.cpp | 1 - static/templates/search_result.html | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/search_renderer.cpp b/src/search_renderer.cpp index 2b55dd902..86718c426 100644 --- a/src/search_renderer.cpp +++ b/src/search_renderer.cpp @@ -97,7 +97,6 @@ std::string SearchRenderer::getHtml() result.set("snippet", it.getSnippet()); std::string zim_id(it.getZimId()); result.set("resultContentId", mp_nameMapper->getNameForId(zim_id)); - std::shared_ptr archive; if (!mp_library) { result.set("bookTitle", kainjow::mustache::data(false)); } else { diff --git a/static/templates/search_result.html b/static/templates/search_result.html index 132348223..b52c9c3d3 100644 --- a/static/templates/search_result.html +++ b/static/templates/search_result.html @@ -125,9 +125,9 @@ {{#snippet}} {{>snippet}}... {{/snippet}} - {{#bookTitle}} -
from {{bookTitle}}
- {{/bookTitle}} + {{#bookTitle}} +
from {{bookTitle}}
+ {{/bookTitle}} {{#wordCount}}
{{wordCount}} words
{{/wordCount}} From cc3545ac3b2718002aa2f9607462a52ded32356d Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Fri, 4 Mar 2022 17:07:41 +0100 Subject: [PATCH 5/6] fixup! Readd a `SearchRenderer` constructor without `Library` argument. --- include/search_renderer.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/search_renderer.h b/include/search_renderer.h index b16567ee1..627699a69 100644 --- a/include/search_renderer.h +++ b/include/search_renderer.h @@ -49,8 +49,8 @@ class SearchRenderer /** * Construct a SearchRenderer from a SearchResultSet. * - * The constructed version of the SearchRendered will not introduce - * the book name for each result. It is better to use the another constructor + * The constructed version of the SearchRenderer will not introduce + * the book name for each result. It is better to use the other constructor * with a Library pointer to have a better html page. * * @param srs The `SearchResultSet` to render. From 422f4c7dd7eb547e6af1578e97c88b94b4df05b0 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Fri, 4 Mar 2022 17:08:59 +0100 Subject: [PATCH 6/6] Reuse constructor when creating the SearchRenderer with basic constructor. --- src/search_renderer.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/search_renderer.cpp b/src/search_renderer.cpp index 86718c426..a899a1704 100644 --- a/src/search_renderer.cpp +++ b/src/search_renderer.cpp @@ -39,12 +39,12 @@ namespace kiwix /* Constructor */ SearchRenderer::SearchRenderer(Searcher* searcher, NameMapper* mapper) - : m_srs(searcher->getSearchResultSet()), - mp_nameMapper(mapper), - protocolPrefix("zim://"), - searchProtocolPrefix("search://?"), - estimatedResultCount(searcher->getEstimatedResultCount()), - resultStart(searcher->getResultStart()) + : SearchRenderer( + searcher->getSearchResultSet(), + mapper, + nullptr, + searcher->getEstimatedResultCount(), + searcher->getResultStart()) {} SearchRenderer::SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper,