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

Searching for integers or version numbers does not work #1773

Closed
3 tasks done
michael-nok opened this issue Jun 22, 2020 · 8 comments
Closed
3 tasks done

Searching for integers or version numbers does not work #1773

michael-nok opened this issue Jun 22, 2020 · 8 comments
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open

Comments

@michael-nok
Copy link
Contributor

michael-nok commented Jun 22, 2020

I checked that...

  • ... the documentation does not mention anything about my problem
  • [] ... the problem doesn't occur with the default MkDocs template
  • ... the problem is not in any of my customizations (CSS, JS, template)
  • ... there are no open or closed issues that are related to my problem

Description

When searching for numbers, either on their own, or as part of a word (e.g. SHA256), the search function does not yield any results.

Expected behavior

I would expect that, by default, searching for numbers (integers) would be supported. In fact, it seems like it used to be supported, but has been removed or disabled.

Steps to reproduce the bug

  1. Visit https://squidfunk.github.io/mkdocs-material/releases/changelog/ and search for "2020" or "5.3.2". No results will be found.
  2. Visit https://square.github.io/okhttp/changelog/ and search for "2020" or "4.7.1" and the search function will find results.

On my own installation of Material for Mkdocs, I've tested modifying the search.config.pipeline to remove the trimmer and stopWordFilter, but nothing seems to impact the behavior of looking for numbers or digits with decimal points.

@squidfunk
Copy link
Owner

OkHTTP is Running Material 4.x, so I guess this is a regression as there’s no reason why searching for numbers shouldn’t work. It might be related to the transform function that is applied before the query is sent to lunr.js.

@squidfunk squidfunk added the bug Issue reports a bug label Jun 22, 2020
@squidfunk
Copy link
Owner

squidfunk commented Jun 23, 2020

Fixed in a3c03ae. Now that was a super funny bug. It's coming from the search transform's first replacement, which removes rogue control characters:

.replace(/(?:^|\s+)[*+-:^~]+(?=\s+|$)/g, "")

The - is not escaped, so the left and right characters, namely + and : are now forming a range from +-:, which includes the characters +,-./0123456789:. Everything within this range is filtered from the query, as long as it is either at the beginning of the query or preceded by whitespace and followed by the end of the query or whitespace. 😁 This is now fixed:

.replace(/(?:^|\s+)[*+\-:^~]+(?=\s+|$)/g, "")

Bildschirmfoto 2020-06-23 um 08 50 46

@squidfunk squidfunk added the resolved Issue is resolved, yet unreleased if open label Jun 23, 2020
@michael-nok
Copy link
Contributor Author

Amazing, thank you so much for fixing this! I'm glad you enjoyed the challenge :)

@squidfunk
Copy link
Owner

Released as part of 5.3.3

@michael-nok
Copy link
Contributor Author

michael-nok commented Jun 25, 2020

I installed 5.3.3, and the bug does not seem to be fixed completely, or else the behaviour is not quite what I expected.
mkdocs-material 5.3.3

  • If I search for numbers, they aren't highlighted as part of the search identification (i.e. showing what's being matched).
  • Using a period seems to break the query. This might be an incorrect configuration on my end. Do I need to include "." as part of the search.separator list, or keep it off the list?

It looks like this incorrect range "+-:" is used in other files:
highlighter/index.ts
74: .replace(/[\s*+-:~^]+/g, " ")

transform/index.ts
52: .replace(/(?:^|\s+)[*+-:^~]+(?=\s+|$)/g, "")

And in the document cleanup, the "." is being stripped out.
document/index.ts
83: .replace(/\s+(?=[,.:;!?])/g, "")

This may be removing valid characters. When looking for Docker tag versions, users may want to include the ":" character.
For example: docker pull squidfunk/mkdocs-material:5.3.3

@squidfunk
Copy link
Owner

  • If I search for numbers, they aren't highlighted as part of the search identification (i.e. showing what's being matched).

Yes, it should be highlighted.

  • Using a period seems to break the query. This might be an incorrect configuration on my end. Do I need to include "." as part of the search.separator list, or keep it off the list?

What do you mean by "breaks"? Try searching for "5.1.2" on the official docs - everything works fine including type-ahead.

It looks like this incorrect range "+-:" is used in other files:
highlighter/index.ts
74: .replace(/[\s*+-:~^]+/g, " ")

Oh yes, I guess I missed the other occurrences! Feel free to submit a PR, so we can get highlighting back to work. Using a . works for me on the official docs.

And in the document cleanup, the "." is being stripped out.
document/index.ts
83: .replace(/\s+(?=[,.:;!?])/g, "")

The regex does only apply to those characters if they are preceded by whitespace, i.e. \s+.

@michael-nok
Copy link
Contributor Author

  • Using a period seems to break the query. This might be an incorrect configuration on my end. Do I need to include "." as part of the search.separator list, or keep it off the list?

What do you mean by "breaks"? Try searching for "5.1.2" on the official docs - everything works fine including type-ahead.

Maybe I'm doing something wrong, but if I leave the config.separator as the default value ( "[\s-]+" ) in partials/language/en.html I am able to look for something like "v1.0.0"

However, if I include "." as a separator ( "[\s-.]+" ), this functionality breaks. Am I doing something wrong?
Reference: https://www.mkdocs.org/user-guide/configuration/#separator

@squidfunk
Copy link
Owner

[\s-.]+ should be [\s\-.]+

squidfunk pushed a commit that referenced this issue Jun 26, 2020
Additional fix for #1773 to include highlighting integers.
squidfunk pushed a commit that referenced this issue Jun 26, 2020
* Further updates for #1773

* Update search.9b3611bd.min.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open
Projects
None yet
Development

No branches or pull requests

2 participants