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: Improved autocompletion #135

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

uptickmetachu
Copy link

@uptickmetachu uptickmetachu commented Aug 17, 2024

Hi first time contribution here :)

My typescript is also bad so please be gentle and suggest feedback where neccessary!

Love quickwit but wanted to try a small PR to see how easy it was to make changes to the plugin and hopefully contribute some more changes in the future.

Motivation

Autocompletion is great but two things bugged me:

  1. When completing between two quotes; eg: span_attributes.filename:"CURSOR_IS_HERE" the suggestions will be surrounded by quotes. When accepting the suggestion, the final quote is inserted before the ending quote and as such we end up with incorrect syntax of span_attributes.filename:"SUGGESTION"" <--extra quote
  2. Autocompletion is limited to the first 100 results. This PR uses the field value to prefix search the terms. We need this when we are searching across a field with thousands of terms but the current implementation will never return any result beyond the initial 100 (sorted alphabetically).

@uptickmetachu uptickmetachu changed the title feat/improved autocomplete feat: Improved autocompletion Aug 17, 2024
@fmassot
Copy link
Contributor

fmassot commented Aug 17, 2024

Hey @uptickmetachu! thanks for your contribution! Let me review it and get back to you.

src/datasource/base.ts Outdated Show resolved Hide resolved
src/datasource/utils.ts Outdated Show resolved Hide resolved
@@ -41,6 +40,12 @@ export function LuceneQueryEditor(props: LuceneQueryEditorProps){
if (!word){ return null }
suggestions = await autocompleter(word?.text);
if (suggestions && suggestions.options.length > 0 ) {
// Fixes autocompletion inserting an extra quote when the cursor is before a quote
Copy link
Contributor

Choose a reason for hiding this comment

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

does it make sense to only check that last character?
I don't understand the slice with context.pos. Can you explain exactly what we want to check here?

Copy link
Author

Choose a reason for hiding this comment

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

does it make sense to only check that last character?
I've modified it to check for all characters following the cursor for a double quote instead of just the immediate character.

"quickwit-indexing/src/actors/indexing_pipeline.rs"

Some examples of autocompletion where ↑ is the current cursor position.:

  1. Example of completing immediately behind a double quote: events.event_attributes.code.filepath:"quickwit-indexing/src/actors/↑"
  • The returned completion would omit the final quote "quickwit-indexing/src/actors/indexing_pipeline.rs.
  • Applying the completion would give us: events.event_attributes.code.filepath:"quickwit-indexing/src/actors/indexing_pipeline.rs"
  • The current behaviour would instead give us: events.event_attributes.code.filepath:"quickwit-indexing/src/actors/indexing_pipeline.rs""
  1. Example of completing without quotes: events.event_attributes.code.filepath:↑
  • The returned completion would be surrounded by quotes "quickwit-indexing/src/actors/indexing_pipeline.rs".
  • Applying the completion would give us: events.event_attributes.code.filepath:"quickwit-indexing/src/actors/indexing_pipeline.rs"
  1. Example of completing with whitespace before double quotes: events.event_attributes.code.filepath:"quickwit-indexing/src/actors/↑ "
  • The returned completion would omit the final quote "quickwit-indexing/src/actors/indexing_pipeline.rs.
  • Applying the completion would give us: events.event_attributes.code.filepath:"quickwit-indexing/src/actors/indexing_pipeline.rs " which isn't ideal but is better than an extra double quote

Copy link
Contributor

@fmassot fmassot left a comment

Choose a reason for hiding this comment

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

Can you check the comments? Thanks!

@uptickmetachu
Copy link
Author

Happy to add a Loom/video recording of the changes in action as well if that helps with the review.

@fmassot
Copy link
Contributor

fmassot commented Aug 21, 2024

Not needed, will review it today! thanks!

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.

2 participants