Search results with statistics #2061
Conversation
…s-with-Statistics
Don't really like this display though.
It is not a good approach because it is generating the statistics twice.
Fixed highlighting words in a fuzzy or proximity search and code clean up.
|
|
This code needs plenty of love (I thought the label filtering code didn't need much work but you did a lot on it... so I shudder to think what there is for this). I don't have any problems with the code - it has been bug free for me over the last 6 months. Things to do:
|
| @@ -94,6 +94,11 @@ | |||
| android:configChanges="keyboardHidden|orientation|screenSize|locale" | |||
| android:label="@string/search" | |||
| /> | |||
| <activity | |||
| android:name="net.bible.android.view.activity.search.MySearchResults" | |||
There was a problem hiding this comment.
Isn't this temporary (2 search results activities)
There was a problem hiding this comment.
Maybe. I don't understand much about the manifest :)
There was a problem hiding this comment.
I mean, we have now SearchResults, and MySearchResults. I think SearchResults is deprecated by MySearchResults. End result should be that we remove old SearchResults and rename MySearchResults -> SearchResults.
There was a problem hiding this comment.
I am sure i had answered this somewhere else. Yes, MySearchResults is a duplicate. The old one could be removed.
| @@ -232,7 +277,7 @@ class SearchControl @Inject constructor( | |||
| const val TARGET_DOCUMENT = "TargetDocument" | |||
| private const val STRONG_COLON_STRING = LuceneIndex.FIELD_STRONG + ":" | |||
| private const val STRONG_COLON_STRING_PLACE_HOLDER = LuceneIndex.FIELD_STRONG + "COLON" | |||
| const val MAX_SEARCH_RESULTS = 1000 | |||
| const val MAX_SEARCH_RESULTS = 10000 | |||
There was a problem hiding this comment.
I dont think we need to limit it at all. The most hits i got was about 4000 for some common words. The search itself is very fast. Even big searches in its current form were acceptable (but slow) for me. The complete search results are valuable now because of the stats. My suggestions it make it very large.
| private val GENERAL_EPISTLES_COLOR = Color.rgb(0x67, 0xCC, 0x66) // changed 99 to CC to make a little clearer on dark background | ||
| private val REVELATION_COLOR = Color.rgb(0xFE, 0x33, 0xFF) | ||
| private val OTHER_COLOR = ACTS_COLOR | ||
|
|
||
| public const val BOOK_GRID_FLOW_PREFS = "book_grid_ltr" | ||
| public const val BOOK_GRID_FLOW_PREFS_GROUP_BY_CATEGORY = "book_grid_group_by_category" | ||
| private const val TAG = "GridChoosePassageBook" | ||
|
|
||
| fun getBookTextColor(bookNo: Int): Int { |
There was a problem hiding this comment.
Why here? Not used in GridChoosePassageBook activity.
Perhaps need to move all these color-related code somewhere else, if they are used in multiple places.
There was a problem hiding this comment.
I cant recall what i did here. But yes, they are now used in two places and i have other places i would like to use them as well. I think i wrote some functions to get what i wanted and added them somewhere. And i seem to recall not knowing where the best place to put them would be.
There was a problem hiding this comment.
I think you have moved this and i have incorporated that change.
| private const val bookTabPosition = 1 | ||
| private const val wordTabPosition = 2 | ||
|
|
||
| class MyTimer(val name:String){ |
There was a problem hiding this comment.
- remove debug code before merge
| supportActionBar?.setDisplayHomeAsUpEnabled(true) | ||
| isScriptureResultsCurrentlyShown = searchControl.isCurrentlyShowingScripture | ||
|
|
||
| GlobalScope.launch { |
There was a problem hiding this comment.
I dont know how to run this in the background sorry.
| } | ||
| } | ||
|
|
||
| private suspend fun prepareResults() { |
There was a problem hiding this comment.
most of stuff in Main, only fetching in IO
| } | ||
| isOk = true | ||
| fetchResultsTimer.stop(true) | ||
| } catch (e: Exception) { |
There was a problem hiding this comment.
Not sure what to narrow it to.
| getString(R.string.search_result_count, mSearchResultsHolder!!.size()) | ||
| } | ||
| withContext(Dispatchers.Main) { | ||
| Toast.makeText(this@MySearchResults, msg, Toast.LENGTH_SHORT).show() |
| val bookOrdinal = ((key as Verse).book as BibleBook).ordinal | ||
| var mBibleBook = BibleBook.values()[bookOrdinal] | ||
| var bookNameLong = versification.getLongName(mBibleBook) // key.rootName | ||
| val bookStat = bookStatistics.firstOrNull{it.book == bookNameLong} |
There was a problem hiding this comment.
- It looks like bookStatistics and other statistics containers could be maps, which is more efficient
|
Yes, I wanted to keep the old way so i could easily go back and look
at how it worked. But it is not needed.... except if someone hates the new
search and just wants the simple old search results back. If the results
with Statistics works as fast as the old search then there is no need for
both.
|
|
I took new strings from here to develop already, so that this can be included easily even after 4.1 release if this is not ready before that. I will leave this now waiting for the mentioned improvements (anyone is free to contribute). |
|
I have implemented the 'Show key words only' now. Done very little testing on it. Will do that over the next few weeks. It's rudimentary but seems to give good results. |
Closes #2049