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

Apply highlight tags on numbers (not only strings) #1758

Open
curquiza opened this issue Oct 2, 2021 · 11 comments
Open

Apply highlight tags on numbers (not only strings) #1758

curquiza opened this issue Oct 2, 2021 · 11 comments

Comments

@curquiza
Copy link
Member

@curquiza curquiza commented Oct 2, 2021

Check with our PR @gmourier, this addition can be implemented by the community 🎉


Currently, the search engine does not highlight the number in _formatted because it's impossible to add <em> tag around a number (not a string) in JSON.

What we expect?
Return the raw document (so the number) but the "transformed" document (so with the number surrounded of the tag in a string) in _formatted.

Example:

{
    "hits": [
        {
            "book_id": 456,
            "title": "Le Petit Prince",
            "_formatted": {
                "book_id": "<em>4</em>56",
                "title": "Le Petit Prince"
            }
        }
    ],
    "offset": 0,
    "limit": 20,
    "processingTimeMs": 0,
    "query": "4"
}
@spinales
Copy link

@spinales spinales commented Oct 2, 2021

@curquiza I'll do, please assign it to me.

@curquiza
Copy link
Member Author

@curquiza curquiza commented Oct 3, 2021

Hello @spinales, you are assigned! 🙂

@spinales
Copy link

@spinales spinales commented Oct 3, 2021

Hello @curquiza I'm beginner with rust, can you give a hand, what function process the JSON files?

@curquiza
Copy link
Member Author

@curquiza curquiza commented Oct 4, 2021

@MarinPostma or @irevoire can maybe give you some clarification about the issue 🙂

@irevoire
Copy link
Member

@irevoire irevoire commented Oct 4, 2021

Hello @spinales, I think all your work will be in the meilisearch-lib/src/index/search.rs file.

You should notice that the big perform_search function is executed for every search.
And that at the end of a search it's calling the format_fields on every returned document.
And in the end, it's in the format_value that decide how to highlight each fields.
And I'll let you guess what you need to do from here 👍

I think this should suffice but I may be wrong, do not hesitate to ask more questions if it doesn't work 😄

@spinales
Copy link

@spinales spinales commented Oct 7, 2021

@irevoire Hi, how can test the changes i did?
i did a change, i want to do test, how can generate JSON file with field _formatted, how showed the first comment on this issue?

@irevoire
Copy link
Member

@irevoire irevoire commented Oct 7, 2021

Hi @spinales, when you search for a specific document you can specify the attributesToHighlight.

image

@curquiza
Copy link
Member Author

@curquiza curquiza commented Oct 14, 2021

Hello @spinales, I will remove the assignation since we did not get any PR from you, we can understand this is not an easy PR and you may not have the time to do it 🙂

In general, and from now, we prefer not assigning people to our issues because sometimes people ask to be assigned and never come back, which discourages the other volunteers contributors to open a PR to fix this issue.
We will accept and merge the first PR that fixes correctly and well implements the issue following our contributing guidelines.

thanks again for your involvement and feel free to still open a PR if you have the time 🙂

@spinales
Copy link

@spinales spinales commented Oct 15, 2021

@curquiza Sorry, I think this issue is easy to resolve, but I'm a beginner with Rust, I think that's the first problem. But the function that is used to search on string can be reused, as the number is a string in the end, that's a possible solution.

@Jhnbrn90
Copy link

@Jhnbrn90 Jhnbrn90 commented Oct 15, 2021

Hi @curquiza and @spinales

I was following this thread a bit and worked out a solution on my own, since I'm also new to Rust and this issue is a nice learning exercise.

I submitted my attempt in this PR: #1813

I'm open to suggestions and improvements and would love to hear your feedback.

@curquiza
Copy link
Member Author

@curquiza curquiza commented Oct 16, 2021

Hello!

@spinales, to be honest MeiliSearch is not an easy project to contribute. I recommend you watching the opening PRs and review them if you want to learn more. I think it's a good start to learn 😄

@Jhnbrn90, thank you! One of the core team developer will review it as soon as possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants