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

Smart crop add an extra space in _formatted field #2528

Closed
alallema opened this issue Jun 16, 2022 · 5 comments · Fixed by #2546
Closed

Smart crop add an extra space in _formatted field #2528

alallema opened this issue Jun 16, 2022 · 5 comments · Fixed by #2546
Assignees
Labels
bug Something isn't working as expected milli Related to the milli workspace v0.28.0 PRs/issues solved in v0.28.0
Milestone

Comments

@alallema
Copy link

Describe the bug
The smart crop add an extra space in the _formatted field.

To Reproduce
Steps to reproduce the behavior:

  1. Send a search request with this parameters
{
	"q": "to",
	"attributesToCrop": ["Title"],
	"cropLength": 2,
	"cropMarker": "(ꈍᴗꈍ)"
}
  1. Get this result on the v0.28.0rc0
    "Title": "(ꈍᴗꈍ)Guide to (ꈍᴗꈍ)",
  1. When I had this result with v0.27.1
    "Title": "(ꈍᴗꈍ)Guide to(ꈍᴗꈍ)",

Expected behavior
I'm not sure if this behavior is expected, but even if it is, it's more complicated to remove an extra space than to add one directly in the cropMarker.

Meilisearch version: v0.28.0rc0

@curquiza
Copy link
Member

curquiza commented Jun 16, 2022

Hello @alallema thanks for your issues

@ManyTheFish let us know if it's expected 😇

@curquiza curquiza added this to the v0.28.0 milestone Jun 16, 2022
@ManyTheFish
Copy link
Member

ManyTheFish commented Jun 16, 2022

hey @curquiza and @alallema!
I think that the behavior expectation is fulfilled until we return the good amount of words.
However, I can make a PR forcing the new formater to trim spaces. 😄

@curquiza curquiza added bug Something isn't working as expected milli Related to the milli workspace labels Jun 16, 2022
bors bot added a commit to meilisearch/milli that referenced this issue Jun 20, 2022
559: Avoid having an ending separator before crop marker r=irevoire a=ManyTheFish

related to meilisearch/meilisearch#2528


Co-authored-by: ManyTheFish <many@meilisearch.com>
@bidoubiwa
Copy link
Contributor

With the crop marker it is clearer that it is a bit weird:

…book about a prince that walks …

@curquiza
Copy link
Member

It will be fixed in rc1! 😄

@curquiza curquiza linked a pull request Jun 23, 2022 that will close this issue
@curquiza
Copy link
Member

curquiza commented Jun 23, 2022

Closed by #2546

@curquiza curquiza added the v0.28.0 PRs/issues solved in v0.28.0 label Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected milli Related to the milli workspace v0.28.0 PRs/issues solved in v0.28.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants