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

JAMES-4077 JMAP - SearchSnippet/get method #2442

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

vttranlina
Copy link
Contributor

Copy link
Contributor

@chibenwa chibenwa left a comment

Choose a reason for hiding this comment

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

Looks good!

@vttranlina vttranlina changed the title [WIP] - JAMES-4077 JMAP - SearchSnippet/get method JAMES-4077 JMAP - SearchSnippet/get method Oct 21, 2024
@vttranlina vttranlina marked this pull request as ready for review October 21, 2024 02:37
Copy link
Contributor

@Arsnael Arsnael left a comment

Choose a reason for hiding this comment

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

Read it nothing to add

Copy link
Contributor

@chibenwa chibenwa left a comment

Choose a reason for hiding this comment

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

IMO we have here three tickets at once:

    1. Back memory-app with Lucene
    1. Add Lucene support for attachment search (prerequisite of 1. unless we temporarily abandon attachment search in 1. ?
    1. And implement SearchSnippet/get (we can do this first onto distributed james)

Please split that in 3 PR...

@vttranlina
Copy link
Contributor Author

vttranlina commented Oct 22, 2024

Back memory-app with Lucene

pr: #2464

And implement SearchSnippet/get (we can do this first onto distributed james)

I will force push code to this pr.

Add Lucene support for attachment search

Ticket: linagora#5306

@vttranlina
Copy link
Contributor Author

The commit JAMES-4077 Back memory-app with Lucene need to spend time to update LuceneSearch (ref #2464)

If we want to make this PR independent of it.

I suggest two options:

  1. Install SearchSnippetGet (which requires SearchHighlight) for distributed app only.
  2. Alternatively, we can implement a FakeSearchHighlight.

@quantranhong1999
Copy link
Contributor

Install SearchSnippetGet (which requires SearchHighlight) for distributed app only.

+1

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

Successfully merging this pull request may close these issues.

[SearchSnippet] SearchSnippet/get method
4 participants