Skip to content

Enhancements to painless autocomplete in monaco#85055

Merged
alisonelizabeth merged 4 commits intoelastic:masterfrom
alisonelizabeth:painless_autocomplete_enhancements
Dec 8, 2020
Merged

Enhancements to painless autocomplete in monaco#85055
alisonelizabeth merged 4 commits intoelastic:masterfrom
alisonelizabeth:painless_autocomplete_enhancements

Conversation

@alisonelizabeth
Copy link
Copy Markdown
Contributor

@alisonelizabeth alisonelizabeth commented Dec 4, 2020 •
edited
Loading

This PR addresses various feedback around the Painless autocomplete functionality.

  • Auto closing brackets
  • Autocomplete support for emit and params keywords
    • Note: emit() is only valid in the context of runtime fields
  • Filter out text field types from the field autocomplete suggestions, as text field types are never in doc values
  • Autocomplete should not kick in for string and boolean values

@alisonelizabeth alisonelizabeth added v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes Feature:Painless Lab Dev tool for learning Painless v7.11.0 Project:RuntimeFields labels Dec 4, 2020
This was referenced Dec 4, 2020
Merged
Merged
alisonelizabeth
alisonelizabeth commented Dec 7, 2020
View reviewed changes
packages/kbn-monaco/src/painless/worker/lib/autocomplete_utils.ts
* 1. If the preceding word is a primitive type, e.g., "boolean", we assume the user is declaring a variable name
* 2. If the string contains a "dot" character, we assume the user is attempting to access a property that we do not have information for
* 3. If the user is defining a variable with a boolean type, e.g., "boolean myBoolean ="
* 4. If the user is defining a string
Copy link
Copy Markdown
Contributor Author

@alisonelizabeth alisonelizabeth Dec 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This entire logic should probably be more sophisticated, but I think we will improve upon this once ANTLR is integrated and we are able to provide contextual autocomplete.

@alisonelizabeth alisonelizabeth marked this pull request as ready for review December 7, 2020 14:16
@alisonelizabeth alisonelizabeth requested a review from a team as a code owner December 7, 2020 14:16
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 7, 2020

Pinging @elastic/es-ui (Team:Elasticsearch UI)

jloleysens
jloleysens approved these changes Dec 8, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Great work @alisonelizabeth ! Thanks for refining the autocomplete on boolean/string values!

I tested locally and was able to get param autocomplete and closing pairs. I am not sure how to properly test the "emit" autocomplete (happy for test coverage though) and I was not sure on how to test "text" field type filtering. I tried in the runtime fields painless editor but was not able to see "emit", and not 100% sure what to look for w.r.t. "text" type.

Let me know if you'd like me to retest, otherwise code looks great! Great work on all these Painless improvements 👏🏻

@alisonelizabeth
Copy link
Copy Markdown
Contributor Author

alisonelizabeth commented Dec 8, 2020

Thanks, @jloleysens for the review!

I am not sure how to properly test the "emit" autocomplete (happy for test coverage though) and I was not sure on how to test "text" field type filtering. I tried in the runtime fields painless editor but was not able to see "emit", and not 100% sure what to look for w.r.t. "text" type.

Sorry about that - I just realized the branch had not yet been updated with #84943. It should be good to go now. Regarding the text type, if you have any defined mapped fields with the text field type, they should not appear in the autocomplete suggestions list.

autocomplete

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Dec 8, 2020

💚 Build Succeeded

Metrics [docs]

Distributable file count

id before after diff
default 46948 47708 +760
Unknown metric groups

@kbn/ui-shared-deps asset size

id before after diff
kbn-ui-shared-deps.js 13.5MB 13.5MB +3.2KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@alisonelizabeth alisonelizabeth merged commit 7b36245 into elastic:master Dec 8, 2020
@alisonelizabeth alisonelizabeth deleted the painless_autocomplete_enhancements branch December 8, 2020 17:16
alisonelizabeth added a commit to alisonelizabeth/kibana that referenced this pull request Dec 8, 2020
@alisonelizabeth
Enhancements to painless autocomplete in monaco (elastic#85055)
05fae6b
@alisonelizabeth alisonelizabeth mentioned this pull request Dec 8, 2020
Merged
alisonelizabeth added a commit that referenced this pull request Dec 8, 2020
@alisonelizabeth
Enhancements to painless autocomplete in monaco (#85055) (#85306)
928254d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jloleysens jloleysens jloleysens approved these changes

Assignees

No one assigned

Labels

Feature:Painless Lab Dev tool for learning Painless Project:RuntimeFields release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.11.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alisonelizabeth @elasticmachine @kibanamachine @jloleysens