Search Improvements

[GautamGupta]

[2429] (#1222) generated the problem. Now every search made returns every post. If a forum is selected, then posts from only that forum are displated.

[GautamGupta]

Reported by ​Mr Pelle

[mr_pelle]

Actually it was not [2429]'s fault, but [2414]'s.

Reverting to [2377] fixes the search but (obviously) breaks Relevant Posts' links (both post and author's) again.

[GautamGupta]

I think it was actually Relevant Topics, not Relevant Posts

[mr_pelle]

Replying to GautamGupta:

I think it was actually Relevant Topics, not Relevant Posts

You're right! Relevant query returns topic objects if its type is set to 'topic' so Kakumei search.php must use

foreach ( $relevant as $topic )

instead of

foreach ( $relevant as $bb_post )

Of course post_link(), bb_get_post_time() and post_text() must be changed as well.

I've just completed a simple patch, I will submit it within minutes.

[mr_pelle]

Correct patch: I forgot to revert search.php to [2377]

[mr_pelle]

Correct patch submitted. I forgot to revert search.php to [2377]

[mr_pelle]

Little add to display last poster next to each Recent Post

[mr_pelle]

I've added another (optional) patch to display last poster next to each Recent Post. It comes from a theme of mine, hope you like it. :)

[GautamGupta]

I have made something better, will post after a few minutes.

[mr_pelle]

More details on search results

[mr_pelle]

Last patch I added is what I currently use in my theme. Maybe is too much for a default theme...

[GautamGupta]

The patch does the following:

• Styles the Search

• Also highlights the keywords in the title

• Adds a title

• Adds pagination

• Fixes some translation strings

• Also adds "Posted By" and "Last Post by" (which was submitted in a patch by mr_pelle

• Adds a nice message with suggestions if no result was found

[GautamGupta]

Updated the patch.

[mr_pelle]

Wow, great work Gautam! Can't wait for 1.0.3 to be released!

[mr_pelle]

datetime format topic_time title on links

[mr_pelle]

Consistency patch to give search results links the title attribute with datetime format topic_time like last post links in v1.1-alpha's front page, forum, etc.

[mr_pelle]

Important note: my last patch makes use of 'datetime' format defined in _bb_time_function_return() (in v1.1-alpha) for Relevant Topics links. Unfortunately that function does not check bbPress time zone, like Recent Posts seems to do, making results times differ of your GMT offset...

Any way to fix this? Use bb_get_last_post() and retrieve its posting time instead of just topic_time()?

[GautamGupta]

Hmm.. that function is also used elsewhere (in front-page etc.).

[mr_pelle]

better patch for datetime title

[mr_pelle]

Fixed in my latest patch.

I've also tried to set 'localize' in the query args (see functions.bb-template.php, line 3558), but it didn't work...

[mr_pelle]

According to ​this page, _bb_time_function_return() is referenced 7 times in bbPress source (2 in Kakumei's search.php): are we going to replace all those instances with bb_datetime_format_i18n?

[GautamGupta]

Replying to mr_pelle:

According to ​this page, _bb_time_function_return() is referenced 7 times in bbPress source (2 in Kakumei's search.php): are we going to replace all those instances with bb_datetime_format_i18n?

Please open a new ticket for that.

[mr_pelle]

Replying to GautamGupta:

Replying to mr_pelle:

According to ​this page, _bb_time_function_return() is referenced 7 times in bbPress source (2 in Kakumei's search.php): are we going to replace all those instances with bb_datetime_format_i18n?

Please open a new ticket for that.

Done: #1280.

What about this ticket, now?

[GautamGupta]

I'll tell chrishajer to test and commit the patch.

[mr_pelle]

if #1280 will be committed, just use this to display datetime title

[mr_pelle]

Forget about my last reply (see #1280), is better to use i18n functions to filter dates and times...

[chrishajer]

I suspect this is MORE broken now.

I fixed #1280.

I patched with with1280.diff then I tried 1274.2.diff and got a bunch of errors:

(Stripping trailing CRs from patch.)
patching file bb-includes/functions.bb-formatting.php
(Stripping trailing CRs from patch.)
patching file bb-includes/functions.bb-template.php
(Stripping trailing CRs from patch.)
patching file bb-templates/kakumei/search.php
Hunk #2 FAILED at 24.
Hunk #3 succeeded at 37 (offset -1 lines).
1 out of 3 hunks FAILED -- saving rejects to file bb-templates/kakumei/search.php.rej
(Stripping trailing CRs from patch.)
patching file bb-templates/kakumei/style.css
(Stripping trailing CRs from patch.)
patching file search.php

So I reverted.

I will attach the search files that were unpatched (orig, rej and php)

[chrishajer]

search.php file after trying to apply 1274.2.diff

[chrishajer]

search.php.orig after trying to apply 1274.2.diff

[chrishajer]

search.php.rej after trying to apply 1274.2.diff

[chrishajer]

I reverted my local copy until someone can tell me the correct order to apply these patches.

[sambauers]

Easier if people submit one big patch on the root directory instead of lot's of little ones on each file.

[GautamGupta]

Actually the only patch needed to be applied was 1274.2.diff, and I had sent a mail to chris to test it with with1280.diff before mr_pelle's ​21st comment, which created the problem.

So, you just need to apply 1274.2.diff. It works fine for me, I'll post a screenshot of it's final outcome.

[GautamGupta]

Final Outcome after applying the patch

[mr_pelle]

Replying to GautamGupta:

Actually the only patch needed to be applied was 1274.2.diff, and I had sent a mail to chris to test it with with1280.diff before mr_pelle's ​21st comment, which created the problem.

Actually I ​replied again in #1280 saying that my patch was wrecking all other last post links before chris submitted it...

Anyway, sorry about that...

[mr_pelle]

What about including part/all of #677?

[GautamGupta]

Final Patch - Changes the way time is displayed in Relevant Topics

[GautamGupta]

Added the final patch which makes the relevant topics' time style look like one of recent posts.

[chrishajer]

(In [2436]) Search enhancements. Maybe fixes #1274. Props GautamGupta, mr_pelle

[mr_pelle]

I'm really sorry to say that we still have problems... probably something of Gautam's patch got lost in [2436]:

1) All Relevant Topics links link to last Recent Posts result.

2) All Relevant Topics author links are broken.

3) At line 31 of search.php, I'm pretty sure it should be

if ( $relevant ) {

4) Relevant Topics freshness span text is different from the one you can see ​here.

5) Relevant Topics context is not retrieved because bb_show_context( $q, $topic->post_text ); does not work. We should first retrieve topic first post id and then call "->post_text"

---

Regarding point 4, do you think Relevant Topics links must link to the topic itself or to topic last post?

I'd prefer the former. Thoughts?

I'll start writing 2 different patches, you'll decide which one to use.

[GautamGupta]

Replying to mr_pelle:

4) Relevant Topics freshness span text is different from the one you can see ​here.

I explained that in ​this reply, the format in which time is displayed should be same (in recent and relevant).

And sorry for the bugs in the patch, I don't know how they crept in.

[mr_pelle]

Replying to GautamGupta:

Replying to mr_pelle:

4) Relevant Topics freshness span text is different from the one you can see ​here.

I explained that in ​this reply, the format in which time is displayed should be same (in recent and relevant).

Like this?

Recent Posts

link to post, "Posted by ... on ..."

Relevant Topics

link to topic last post, "Posted by ... on ..."

My patch is almost done...

[mr_pelle]

Imho, if Relevant Topics link to topic last post, their freshness should be "Last post ... ago by ...", with datetime <title/> as front page last posts links.

Don't you think "Posted by ... on ..." on a topic is a little confusing? I mean, does it refer to topic first or last post? It's not that clear...

[mr_pelle]

fixes bugs. Relevant freshness is the same as Recent's

[mr_pelle]

fixes bugs. Relevant freshness is the same as front page last posts'

[mr_pelle]

Choose your favorite, not both! ;)

[GautamGupta]

We are actually linking to the first post of the topic, not to the last post. Attaching another patch which fixes the bugs and also replaces "Posted by" with "By" ("Posted" takes up lot of space and it doesn't look nice).

[GautamGupta]

Fixes the bugs that went into [2436]

[mr_pelle]

Replying to GautamGupta:

We are actually linking to the first post of the topic, not to the last post. Attaching another patch which fixes the bugs and also replaces "Posted by" with "By" ("Posted" takes up lot of space and it doesn't look nice).

Ok, that's right.

From a mere spacing point of view, don't you think <li/> elements inside both Recent and Relevant loops are 1 tab too much on the right?
Also <?php $bb_post = bb_get_first_post( $topic ); ?> should be aligned correctly, being it inside the loop too.

[mr_pelle]

Ignore my last reply: I understood <li/>s indentation refers to <ol/> tag and not to any php...

[mr_pelle]

Uff... bb_search_pages() does not return pages navigation link. I set bbPress "items per page" to 2 and I get only 2 Recents and 2 Relevants.

[GautamGupta]

Replying to mr_pelle:

Uff... bb_search_pages() does not return pages navigation link. I set bbPress "items per page" to 2 and I get only 2 Recents and 2 Relevants.

You need to change 5 everywhere, in bb_search_pages function, and $search_start, $search_stop and per_page in search.php which exists in root folder.

[mr_pelle]

Replying to GautamGupta:

Replying to mr_pelle:

Uff... bb_search_pages() does not return pages navigation link. I set bbPress "items per page" to 2 and I get only 2 Recents and 2 Relevants.

You need to change 5 everywhere, in bb_search_pages function, and $search_start, $search_stop and per_page in search.php which exists in root folder.

Sorry, change 5 into what? bb_get_option('page_topics')?

[mr_pelle]

I managed to get pages navigation link displayed, but it disappears on page 2...

I also added $args to bb_search_pages(), since it was missing (usually used for adding div.nav).

[mr_pelle]

I think there is a problem with $search_count: at the moment it is different from results total, thus it cannot be used as "total" in get_page_number_links().

Example: if we have 3 results, 2 items per page (bbPress option) and we browse to page 2, $search_count will be 1 (thus not showing page navigation link), while it should be 3 in all the pages, allowing bbPress to correctly calculate ceil( $total/$per_page ) (line 1360, functions.bb-template.php).

Pagination should be controlled via bbPress "item per page" option and not hardcoded, imho.

Anyway, at the moment I've no idea how to fix this, but I'll update last patch with some little additions.

[mr_pelle]

updated with page navigation link. Does NOT fix everything yet!

[mr_pelle]

If we could set get_page_number_links()'s "total" (line 1651, functions.bb-template.php) to the correct value (= results total), all problems would solve. I've indeed tried setting it manually and works.

[mr_pelle]

further update. Restores 5 item per page. Does NOT fix everything yet!

[mr_pelle]

Do the 2 new filters make search ignore posts in the same topic??

[GautamGupta]

Includes 2436-fixes.diff, bb_search_pages fix and per_page fix (checks for items per page)

[GautamGupta]

Can you please test 2436-and-per-page-fixes.patch? It includes the fixes in 2436-fixes.diff, bb_search_pages function fixes by Mr. Pelle and items per page fixes.

[mr_pelle]

Replying to GautamGupta:

Can you please test 2436-and-per-page-fixes.patch? It includes the fixes in 2436-fixes.diff, bb_search_pages function fixes by Mr. Pelle and items per page fixes.

I'm on it! :)

[mr_pelle]

Works great except for the 2 new filters that make search ignore posts in the same topic, as I said before. I had to remove them both to get complete results...

If your search string contains exclamation points or question marks, these chars are escaped in the post context (\! \?). This does not happen with ticks, backticks, slashes or backslashes.

Nit-picking, there are some superfluous tabs (lines 1647, 1650 of functions.bb-template.php; lines 16, 28, 38, 40 of search.php) and 2 pairs of braces that could be removed (lines 25, 27, 35, 37 of search.php).

Can you also explain me why you added tabs to lines 46, 47, 48 of kakumei/search.php, please?

[GautamGupta]

Replying to mr_pelle:

Works great except for the 2 new filters that make search ignore posts in the same topic, as I said before. I had to remove them both to get complete results...
If your search string contains exclamation points or question marks, these chars are escaped in the post context (\! \?). This does not happen with ticks, backticks, slashes or backslashes.

Hmm.. would have to look into that.

Nit-picking, there are some superfluous tabs (lines 1647, 1650 of functions.bb-template.php; lines 16, 28, 38, 40 of search.php)

They're fine for me - in my code editor (Komodo).

2 pairs of braces that could be removed (lines 25, 27, 35, 37 of search.php).

I don't like if else statements in which one section has braces and the other doesn't.
Please check ​http://codex.wordpress.org/WordPress_Coding_Standards

Can you also explain me why you added tabs to lines 46, 47, 48 of kakumei/search.php, please?

Those are 8 character space tabs (which are also there in rest of the file). Previous were 4.

[mr_pelle]

Replying to GautamGupta:

Replying to mr_pelle:

Nit-picking, there are some superfluous tabs (lines 1647, 1650 of functions.bb-template.php; lines 16, 28, 38, 40 of search.php)

They're fine for me - in my code editor (Komodo).

I use Komodo (Edit) too. I've always believed that if a line contains no text, spaces and tabs should be removed too.

2 pairs of braces that could be removed (lines 25, 27, 35, 37 of search.php).

I don't like if else statements in which one section has braces and the other doesn't.
Please check ​http://codex.wordpress.org/WordPress_Coding_Standards

I already knew the page, but I forgot that rule, sorry.

Can you also explain me why you added tabs to lines 46, 47, 48 of kakumei/search.php, please?

Those are 8 character space tabs (which are also there in rest of the file). Previous were 4.

If you mean the <li/>s at lines 15-19 and 31-35, their indentation is one 4-spaces-tab on their parent <ol/>'s right, while at lines 46-48 the <ul/> is not indented, so <li/>'s indentation should be just one 4-spaces-tab, not 2. Right?

[GautamGupta]

Replying to mr_pelle:

I use Komodo (Edit) too. I've always believed that if a line contains no text, spaces and tabs should be removed too.

Not necessary, I like them to be there.

If you mean the <li/>s at lines 15-19 and 31-35, their indentation is one 4-spaces-tab on their parent <ol/>'s right, while at lines 46-48 the <ul/> is not indented, so <li/>'s indentation should be just one 4-spaces-tab, not 2. Right?

You can check the ​patch, 1 indent = 8 spaces. At lines 15, 19 and 31, 35 - there are a total of 2 tabs = 16 spaces and the li I edited, is only 1 tab = 8 spaces. Hope that clears your doubt.

[mr_pelle]

Replying to GautamGupta:

Replying to mr_pelle:

If you mean the <li/>s at lines 15-19 and 31-35, their indentation is one 4-spaces-tab on their parent <ol/>'s right, while at lines 46-48 the <ul/> is not indented, so <li/>'s indentation should be just one 4-spaces-tab, not 2. Right?

You can check the ​patch, 1 indent = 8 spaces. At lines 15, 19 and 31, 35 - there are a total of 2 tabs = 16 spaces and the li I edited, is only 1 tab = 8 spaces. Hope that clears your doubt.

Yep, thank you!

I hope we're finally done with this ticket!

[mr_pelle]

Oh, what about the filters?

[chrishajer]

(In [2440]) Trying again to fix pagination in search. Fixes #1274. Props GautamGupta

[mr_pelle]

Replying to GautamGupta:

Replying to mr_pelle:

Works great except for the 2 new filters that make search ignore posts in the same topic, as I said before. I had to remove them both to get complete results...
If your search string contains exclamation points or question marks, these chars are escaped in the post context (\! \?). This does not happen with ticks, backticks, slashes or backslashes.

Hmm.. would have to look into that.

Thank you Chris, but... Gautam, have you checked the filters behavior?

[GautamGupta]

Replying to mr_pelle:

Thank you Chris, but... Gautam, have you checked the filters behavior?

Those have been there for long time. Check [907] #694. Also ​http://trac.bbpress.org/log/trunk/search.php

[mr_pelle]

Replying to GautamGupta:

Replying to mr_pelle:

Thank you Chris, but... Gautam, have you checked the filters behavior?

Those have been there for long time. Check [907] #694. Also ​http://trac.bbpress.org/log/trunk/search.php

Oh, ok...

[chrishajer]

Are the incorrect search results on this page due to this bug or another bug that's been reported?

http://bbpress.org/plugins/search.php?q=approve

None of the links work, and the date and time are the date and time the search was performed (in GMT I think.)

[GautamGupta]

Better way to do pagination

[GautamGupta]

Added another patch which does pagination the better way.

[chrishajer]

Committed that patch in r2449