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

feat: add full-text search #21

Merged

Conversation

AndyOGo
Copy link
Contributor

@AndyOGo AndyOGo commented Jan 16, 2022

[One line description of your change]

Motivation:

Find desired frames quicker.

[Explain here the context, and why you're making that change. What is the problem you're trying to solve.]

Modifications:

This PR adds a simple <input /> field with full-text search powered by flexsearch.

fixes #20

[Describe the modifications you've done.]

Result:

[After your change, what will change.]

Text Frame:
RSocketFrameViewer-Text-frame-search-Jan-17-2022 00-45-52

RSocket Frame data:
RSocketFrameViewer-frame-data-search-Jan-17-2022 00-48-42

@AndyOGo AndyOGo force-pushed the feat/filter-frames-by-full-text-search branch from a161138 to 81149b0 Compare January 16, 2022 23:39
const Row = ({ index, style, data }: ListChildComponentProps) => {
const frame = data[index];
const Row = ({ index, style }: ListChildComponentProps) => {
const frame = filteredFrames[index];
Copy link
Contributor Author

@AndyOGo AndyOGo Jan 16, 2022

Choose a reason for hiding this comment

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

@SerCeMan
I fixed that regression which I caused in my previous PR
#19 (comment)

If you prefer I may provide a separate PR just for that fix?

Copy link
Member

Choose a reason for hiding this comment

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

No worries, I'll do another round of testing before releasing the extension.

@@ -347,6 +371,7 @@ export class AppStateStore {
id: requestId,
url: url,
frames: [],
index: new Index({tokenize: "full"}),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I deemed a "full" search to be the best fit.

index every possible combination
https://github.com/nextapps-de/flexsearch#tokenizer-prefix-search

connection.frames.push(frame)
connection.index.add(frame.id, (rSocketFrame as any)?.data ?? frame.text ?? frame.payload)
Copy link
Contributor Author

@AndyOGo AndyOGo Jan 16, 2022

Choose a reason for hiding this comment

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

In my local tests this works for text frames and rsocket frames data.
Is there any basic thing I missed?

It would be possible to filter for specific frame types or other parts of the protocol, but needs parsing and mapping to human readable names.
https://github.com/rsocket/rsocket/blob/master/Protocol.md

Copy link
Member

@SerCeMan SerCeMan left a comment

Choose a reason for hiding this comment

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

Thanks, @AndyOGo! This looks like an awesome feature 🚀

const Row = ({ index, style, data }: ListChildComponentProps) => {
const frame = data[index];
const Row = ({ index, style }: ListChildComponentProps) => {
const frame = filteredFrames[index];
Copy link
Member

Choose a reason for hiding this comment

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

No worries, I'll do another round of testing before releasing the extension.

@SerCeMan SerCeMan merged commit 1993c76 into rsocket:master Jan 17, 2022
@SerCeMan
Copy link
Member

Hey, @AndyOGo! The new version should be available in the extension store now.

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.

Filter frames with full-text search
2 participants