Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better core search #2286

Merged
merged 51 commits into from Apr 5, 2018
Merged

Better core search #2286

merged 51 commits into from Apr 5, 2018

Conversation

micgro42
Copy link
Collaborator

@micgro42 micgro42 commented Mar 23, 2018

This improves the search functionality of DokuWiki by adding some assistance, adding functionality, improving what information in the results are displayed, adding events and fixing bugs.

New Events

This pull request adds the following new events to DokuWiki:

Changed Events

Breaking Changes

ToDo:

  • Filter by lastmod time, closes Feature Request: restrict search result to a time / date range  #1973 (still needs custom date entry)
  • Option to sort by lastmod time instead of number of hits
  • allow easier filtering by namespace (from the new search form and from the results)
  • allow easier changing the fragment behavior (exact match, starts with, ends with, contains)
  • add config option to limit the search to the current X namespaces (when a search is executed from a page within a deeper namespace, the first X namespaces will be added as filter)
  • add config option to specify the default fragment search behavior
  • events for rendering every search result
  • event for the new search form
  • handle namespaces with - and . correctly, fixes Wiki search fails if restricted to a namespace whose ID contains '-' or '.' #1659
  • handle namespaces that are prefixes of other namespaces correctly, fixes Search limitation by namespace matches as prefix #2285
  • using a new q url-parameter for the search query and remain on the current page by not changing id (implemented backwards-compatible). This effectively resolves Parse search-string and remove parameters before setting $ID #1124
  • more refactoring
  • all display-strings must be translatable
  • better styling
    • ARIA labels for dropdowns
    • Toggle Button Styling
    • RTL Styling
    • refactor search.less
  • check behaviour when JS is disabled
  • adjust intro texts in all languages (delete if necessary)
  • documentation on dokuwiki.org

This commit should be without functional changes.
Add the search form on the results page itself. This form will be used
to add more options to refine the search further.
This add some search assistance to simple, single-word search queries
which may be restricted to a single namespace.

Further improvements:
* better styling
* trigger events for other plugins
* set namespaces directly from fulltext search results
* some more config options
Add events around each search result, both for the pagename results
and the fullpage results.

The fullpage results are wrapped in a div for better separation and
styling.
( see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dl )

These events are intended to provide plugin authors with the ability
to hydrate the search results with more information.
Plugins may want to offer more ways to filter the search results.
To allow creating links with manipulated versions of the current search
query an unparser is necessary. However, the current output of ft_queryParser
makes some advanced features hard to detect. Therefore the new
ft_queryUnparser_simple cannot handle negated phrases and `OR` searches.

It should still cover 98% of search queries.
This makes it easier to read, reason and extend.
There are several use cases in which knowing the page where a search
request originated would be useful. This commit adds a `from` parameter
which provides that information.
This adds two new config options:

`search_limit_to_first_ns`:
Limit the search to the current X namespaces. When a search is executed
from a page within a deeper namespace, the first X namespaces will be
added as filter.
Possible use case could be with language namespaces to ensure that the
default search is initially within the current language.

`search_default_fragment_behaviour`:
Option to specify the default fragment search behavior
This simplifies many aspects. However, it still needs much better
styling.
Quick-search positioning and other templates break when removing this
tag.
This fixes a bug that during a search limited to `@de` would show pages
form the namespace `devel` as well (in the fullpage results).

Fixes #2285
This allows filtering of results by the last modified time. It still
needs a custom date entry option, highlighting of the currently selected
option (if any) and a better place where the filtering happens.
@micgro42 micgro42 self-assigned this Mar 23, 2018
@micgro42
Copy link
Collaborator Author

Maybe we should look into #1124 as well?

This way we do not loose the context of the current page. Further, the
new id generated from the query before wasn't really that useful (see
issue #1124 ). And we can still generate a meaningful link "create a
page for your query", if that is considered useful.

Further redirects exist for the old urls, so bookmarked searches etc.
should mostly keep working.
The regex for the pagename lookup didn't account for `-` and `.` being
valid characters for namespaces, which lead to wrong results in the
quicksearch and pagename lookup. The full search, which already used the
queryParser, showed the correct results.

This fixes #1659
This is the more appropriate place for that functionality, because now
it happens inside the default function for the respective pagesearch and
pagelookup events and can be properly handled by plugins.
Building the new search links is complex and we're going to add another
parameter with the new sorting by mtime. Extracting a new class seems
like the cleanest way to handle that increasing complexity.
This functionality is inspired by what other large search engines are
doing.
This gives create control over when a query is too complex for tool
support.
If the entered query does not cleanly fall into the area of the given
options, show the current behavior as "custom".
This way they can be cleanly influenced by the plugins instead of having
to modify global state.
Since we no longer change the page during a search, using the "show"
menu item to return to the current page makes more sense.
Pressing enter in the small quick-search-box while it was empty resulted
in a search for the current pageid.
This is useful for anonymously useable forms, like the search.
$searchForm->setHiddenField('after', $INPUT->str('after'));
$searchForm->setHiddenField('sf', '1');
if ($INPUT->has('dta')) {
$searchForm->setHiddenField('dta', $INPUT->str('dta'));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dta and dtb, this sound very cryptic... and b become chronologically before a, while alphabetically after.

So: for me these shortcuts are to cryptic... I would prefer the possibility to understand something from its name, this gives really no clue.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll rename them to min and max

@splitbrain splitbrain added this to the 🐱 Greebo milestone Mar 28, 2018
@micgro42 micgro42 changed the title [WIP] Better core search Better core search Mar 28, 2018
They are now named min and max respectively, which is hopefully less
cryptic.
@micgro42 micgro42 merged commit 5c0b2e6 into master Apr 5, 2018
@micgro42 micgro42 deleted the betterCoreSearch branch April 5, 2018 07:51
@micgro42 micgro42 mentioned this pull request Apr 5, 2018
4 tasks
micgro42 added a commit that referenced this pull request Apr 5, 2018
This should give us some backwards compatibility for changing files in
dokuwiki template from .css to .less

This way template authors get a warning to adjust their template, it
won't break right away.

This should prevent breaking search pages in other templates due to
pull request #2286 .
@selfthinker selfthinker mentioned this pull request Apr 15, 2018
@selfthinker
Copy link
Collaborator

This PR introduced a lot of invalid HTML which I've fixed in #2314.
Is there a way to let HTML validity be checked at the CI stage?

Also, I wonder why you chose to build a select box from scratch and not use a native one? This means that the advanced options are not very accessible right now. All the ARIA built in don't matter much as none of the options are keyboard accessible. If you had chosen native select boxes, you would have got an accessible version for free.

Another less important thing is that I find the @namespace next to the page name confusing. It is not clear what it does until you try it out. It could at least get a title which clarifies that, so the links make sense out of context for accessibility and usability reasons, or instead of @namespace it could say 'search within "namespace" namespace'.
It does the very same thing as the "any namespace" dropdown, right?

micgro42 added a commit that referenced this pull request Apr 17, 2018
In #2286 the 2nd level heading in the intro was removed, so the h3
should be promoted to h2.
ssahara added a commit to ssahara/dokuwiki that referenced this pull request Aug 26, 2020
FORM_*_OUTPUT scheme had been introduced since Apriil 2018, see dokuwiki#2286
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment