-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Console Monaco] Resolve uncaught error from tokenizer #188746
[Console Monaco] Resolve uncaught error from tokenizer #188746
Conversation
/ci |
💛 Build succeeded, but was flaky
Failed CI StepsTest FailuresMetrics [docs]Page load bundle
cc @ElenaStoeva |
Pinging @elastic/kibana-management (Team:Kibana Management) |
matchTokensWithEOL('method', /([a-zA-Z]+)/, 'root', 'method_sep'), | ||
matchTokensWithEOL( | ||
'method', | ||
/([Gg][Ee][Tt])|([Pp][Oo][Ss][Tt])|([Pp][Uu][Tt])|([Pp][Aa][Tt][Cc][Hh])|([Dd][Ee][Ll][Ee][Tt][Ee])|([Hh][Ee][Aa][Dd])/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit) this regex is somewhat harder to read: we could either put it in a named variable or change the regex to /get|post|put|patch|delete|head/
and add the option ignoreCase: true
to the rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good call! I wasn't sure if adding ignoreCase: true
wouldn't affect the rest of the rules but now I checked and it seems none of the rules rely on strict case sensitivity so it should work. I added this change with 95e7d93.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks a lot for fixing this error, @ElenaStoeva!
💚 Build Succeeded
Metrics [docs]Page load bundle
To update your PR or re-run it, just comment with: cc @ElenaStoeva |
* master: (3487 commits) `BedrockChat` & `GeminiChat` (elastic#186809) [ResponseOps] log error when ES Query rules find docs out of time range (elastic#186332) skip flaky suite (elastic#188997) [Security solution][Alert Details] Enable preview feature flag and cypress tests (elastic#188580) [EuiProviders] Warn Developer if EuiProvider is missing (elastic#184608) [Security Solution ] Fixes Timeline infinite loading bug (elastic#188943) Improve SearchSource SearchRequest type (elastic#186862) Deprecate Search Sessions config (elastic#188037) [Synthetics] Add missing monitorType and tag info in cards !! (elastic#188824) [Console Monaco] Resolve uncaught error from tokenizer (elastic#188746) [Data Forge] Add `service.logs` dataset as a data stream (elastic#188786) [Console] Fix failing bulk requests (elastic#188552) Update dependency terser to ^5.31.2 (main) (elastic#188528) [APM][ECO] Telemetry (elastic#188627) [Fleet] Fix uninstall package validation accross space (elastic#188749) Update warning on `xpack.fleet.enableExperimental` (elastic#188917) [DOCS][Cases] Automate more screenshots for cases (elastic#188697) [Fleet] Fix get one agent when feature flag disabled (elastic#188953) chore(investigate): Add investigate-app plugin from poc (elastic#188122) [Monaco Editor] Add Search functionality (elastic#188337) ...
* master: (2400 commits) `BedrockChat` & `GeminiChat` (elastic#186809) [ResponseOps] log error when ES Query rules find docs out of time range (elastic#186332) skip flaky suite (elastic#188997) [Security solution][Alert Details] Enable preview feature flag and cypress tests (elastic#188580) [EuiProviders] Warn Developer if EuiProvider is missing (elastic#184608) [Security Solution ] Fixes Timeline infinite loading bug (elastic#188943) Improve SearchSource SearchRequest type (elastic#186862) Deprecate Search Sessions config (elastic#188037) [Synthetics] Add missing monitorType and tag info in cards !! (elastic#188824) [Console Monaco] Resolve uncaught error from tokenizer (elastic#188746) [Data Forge] Add `service.logs` dataset as a data stream (elastic#188786) [Console] Fix failing bulk requests (elastic#188552) Update dependency terser to ^5.31.2 (main) (elastic#188528) [APM][ECO] Telemetry (elastic#188627) [Fleet] Fix uninstall package validation accross space (elastic#188749) Update warning on `xpack.fleet.enableExperimental` (elastic#188917) [DOCS][Cases] Automate more screenshots for cases (elastic#188697) [Fleet] Fix get one agent when feature flag disabled (elastic#188953) chore(investigate): Add investigate-app plugin from poc (elastic#188122) [Monaco Editor] Add Search functionality (elastic#188337) ...
Fixes #186765
Summary
This PR fixes the uncaught error from the tokenizer when we add the input below:
Test input
How to test: Add the input above in Console editor and verify that there is no uncaught error in the browser error.
This change also resolves some highlighting inconsistencies, where some text was incorrectly highlighted as a
method
andurl
tokens:Before:
Now: