Make Console Script Aware #9433

bleskes · 2016-12-09T20:56:22Z

At the moment, Console understands HTTP methods, URLs and JSON. However ES has long had the options to write inline scripts in its API and with the introduction of painless inline scripts become more and more popular. Sadly, writing them in JSON is a PITA as you must escape every single thing.

This PR adds the notion of literal strings to Console's flavor of JSON. This means that everything that is surrounded with """ will be JSON escaped before sending to ES. On top of that, if a literal string is found after "script" or "inline" tag it is treated as a script literal, which adds syntax highlighting and some Ace goodness. Script literal are a full blown Ace Mode, meaning we can extend it to do auto complete and similar in the future. Also, auto formatting works as well, automatically taking strings with more than 2 escaped characters and converting them literal strings. In similar fashion, the output pane also auto converts strings in the output.

This is how it all looks:

Script literal:

String literals as fields in documents:

To illustrate what happens when things are sent to ES, here is the Copy as Curl output of the last document:

curl -XPUT "http://localhost:9200/hockey/player/test" -d'
{
  "name": "player1",
  "description": "  some long and explicit text with new lines, \\slashes\\ and\n  {crazy} brackets :/",
  "field_chars": """/regex/g"""
}'

bleskes · 2016-12-09T20:57:36Z

PS. These the keywords currently highlighted in script mode:

let painlessKeywords = (
  "def|int|long|byte|String|new|null|for|if|return|ctx"
);

@nik9000 @jdconrad it would be great if you can extend this list (just post it here and I'll update the code)

nik9000 · 2016-12-09T21:10:29Z

(just post it here and I'll update the code)

Here is the master list: https://github.com/elastic/elasticsearch/blob/master/modules/lang-painless/src/main/antlr/PainlessLexer.g4#L43-L57

nik9000 · 2016-12-09T21:12:04Z

Once we get this in I can modify the tool that extract CONSOLE syntax out of Elasticsearch's docs and we can use this in the docs. Much awesome!

nik9000 · 2016-12-09T21:18:54Z

float and double and char are all missing primitives. That list I pasted above is really just "syntax". null and true and false are also in the list, just hiding below.

jdconrad · 2016-12-09T21:22:28Z

@bleskes This is awesome! @nik9000 Do you think it would be worth somehow special casing all of the types in the whitelist as well? If we could somehow build this to do the recommendations for methods off of a class type that would be pretty amazing.

nik9000 · 2016-12-09T21:24:12Z

If we could somehow build this to do the recommendations for methods off of a class type that would be pretty amazing.

That'd be cool. We could add an endpoint that you could use to look them up if we wanted to get fancy.

epixa · 2016-12-21T21:44:39Z

src/core_plugins/console/public/src/sense_editor/mode/script.js

+    this.$outdent.autoOutdent(doc, row);
+  };
+
+  // this.createWorker = function (session) {


If this code isn't in use, just remove it rather than commenting it out.

left overs indeed. removed.

epixa · 2016-12-21T21:53:17Z

src/core_plugins/console/public/src/sense_editor/mode/input.js

@@ -20,6 +21,9 @@ var Mode = function () {
  this.$outdent = new MatchingBraceOutdent();
  this.$behaviour = new CstyleBehaviour();
  this.foldingRules = new CStyleFoldMode();
+  this.createModeDelegates({
+      "script-": ScriptMode
+    });


Can you remove the extra indents here?

epixa · 2016-12-21T21:53:50Z

src/core_plugins/console/public/src/sense_editor/mode/input.js

-    if (state != "double_q_string") {
+    if (state == "string_literal") {
+      // noop
+    } else {


Since this is a noop, why not just if (state !== 'string_literal') {

epixa · 2016-12-21T22:02:32Z

src/core_plugins/console/public/src/input.js

-  uiModules.get('app/sense').setupResizeCheckerForRootEditors($el, input, output);
+  // this may not exist if running from tests
+  let appSense = uiModules.get('app/sense');
+  if (appSense.setupResizeCheckerForRootEditors) {


Can we mock this in the tests rather than altering the code to accommodate the tests?

I removed the extra check and everything seems to work fine now. I think it's a left over from old times.

I take it back. Things break. I mocked it instead.

epixa · 2016-12-21T22:08:51Z

src/core_plugins/console/public/src/sense_editor/mode/script.js

+  this.$behaviour = new CstyleBehaviour();
+  this.foldingRules = new CStyleFoldMode();
+};
+oop.inherits(ScriptMode, TextMode);


I'm not familiar with the oop module in ace, but you can now use native JS class syntax, which is a little easier to grok:

class ScriptMode extends TextMode { constructor() { this.$outdent = new MatchingBraceOutdent(); this.$behaviour = new CstyleBehaviour(); this.foldingRules = new CStyleFoldMode(); } get HighlightRules() { return ScriptHighlightRules; } getNextLineIndent(state, line, tab) { let indent = this.$getIndent(line); const match = line.match(/^.*[\{\[]\s*$/); if (match) { indent += tab; } return indent; } // etc etc }

I just noticed the acequire thing... I really have no idea what any of that is doing, so take what I just proposed with a grain of salt.

the current ace is a stand alone fully embeddable version of it that tries to isolate itself from the universe. It has it's own version of require that is fully served from a single big file. I think at some point we should move to an unpacked version of ace where it's easier to read the source and we just import ourselves. I rather not tackle it now though.

epixa · 2016-12-21T22:10:16Z

src/core_plugins/console/public/src/sense_editor/mode/script.js

+
+}).call(ScriptMode.prototype);
+
+module.exports.ScriptMode = ScriptMode;


You can use native JS exports now rather than CommonJS module exports, so your class definition can simply be:

export class ScriptMode extends TextMode {

And then you can remove module.exports.ScriptMode = ScriptMode; entirely.

thx for the export tip. applied.

epixa · 2016-12-21T22:14:33Z

src/core_plugins/console/public/src/sense_editor/mode/script_highlight_rules.js

+  "def|int|long|byte|String|new|null|for|if|return|ctx"
+);
+
+var ScriptHighlightRules = function () {


I have the same suggestion here about using native class syntax as I did in the previous file.

I took over the direct export

epixa · 2016-12-21T22:19:35Z

I added a couple of notes. PRs for new features on console are particular challenging because we haven't yet updated the console codebase to be consistent in terms of code style and technologies with the rest of Kibana. I created two issues in particular for this: #9603 #9604

We shouldn't block this PR on those issues, but I wanted to highlight this for both you @bleskes and for other reviewers as we have to tread the fine line between writing code that's consistent with Kibana and code that's consistent with Console.

bleskes · 2016-12-23T16:17:17Z

thx @epixa . I addressed all your comments. Can you take another look?

epixa · 2016-12-23T17:40:20Z

@bleskes Looks like there's a linter failure in CI:

/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-intake/src/core_plugins/console/public/tests/src/utils_tests.js
  6:6   error  A space is required after '{'   babel/object-curly-spacing
  6:64  error  A space is required before '}'  babel/object-curly-spacing

epixa · 2016-12-23T19:05:01Z

@bleskes How do I run the qunit tests for console?

epixa · 2016-12-23T19:06:05Z

Nevermind, I found it.

Edit: Just access http://localhost:5601/app/sense-tests for any future readers.

epixa · 2016-12-23T19:07:33Z

LGTM

Backports PR #9433 **Commit 1:** Add console scripts * Original sha: c645975 * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-09T16:28:26Z **Commit 2:** Merge remote-tracking branch 'upstream/master' into console_scripts * Original sha: 795fcc0 * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-09T16:29:04Z **Commit 3:** Better handling of auto formatting * Original sha: cf6e0b1 * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-09T20:15:16Z **Commit 4:** Merge remote-tracking branch 'upstream/master' into console_scripts * Original sha: 036b0f6 * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-23T15:17:52Z **Commit 5:** review feedback * Original sha: 361429e * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-23T16:16:32Z **Commit 6:** linting * Original sha: 412633d * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-23T18:22:28Z

bleskes · 2016-12-23T19:13:10Z

@bleskes Looks like there's a linter failure in CI:

That's peculiar because I thought you guys have linting setup as precommit. Anyway it works now :)

@epixa thx for the review. It seems you guys have a different process (as you merged it in) and it looks like you guys are also taking care of backporting? If so thanks for that as well :)

epixa · 2016-12-23T19:15:18Z

@bleskes To be honest, we do usually expect the submitter to merge/backport, but I didn't know what process you followed, so I treated this as I would a community submitted PR instead. I'll remember this for your future PRs though :P

bleskes · 2016-12-23T19:16:00Z

@epixa ❤️

epixa · 2016-12-23T19:16:54Z

Oh, and as for the precommit hook: the task was broken for a little while and wasn't linting properly, but that was fixed recently. Your branch just didn't have the latest master, so the linter wasn't working.

Backports PR #9433 **Commit 1:** Add console scripts * Original sha: c645975 * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-09T16:28:26Z **Commit 2:** Merge remote-tracking branch 'upstream/master' into console_scripts * Original sha: 795fcc0 * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-09T16:29:04Z **Commit 3:** Better handling of auto formatting * Original sha: cf6e0b1 * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-09T20:15:16Z **Commit 4:** Merge remote-tracking branch 'upstream/master' into console_scripts * Original sha: 036b0f6 * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-23T15:17:52Z **Commit 5:** review feedback * Original sha: 361429e * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-23T16:16:32Z **Commit 6:** linting * Original sha: 412633d * Authored by Boaz Leskes <b.leskes@gmail.com> on 2016-12-23T18:22:28Z

bleskes added 3 commits December 9, 2016 17:28

Add console scripts

c645975

Merge remote-tracking branch 'upstream/master' into console_scripts

795fcc0

Better handling of auto formatting

cf6e0b1

epixa added Team:Operations Team label for Operations Team release_note:enhancement review labels Dec 9, 2016

epixa assigned tylersmalley and epixa Dec 9, 2016

tbragin added the Feature:Console Dev Tools Console Feature label Dec 14, 2016

epixa mentioned this pull request Dec 21, 2016

[webpack] upgrade to @elastic/webpack-directory-name-as-main v2 #9601

Merged

epixa reviewed Dec 21, 2016

View reviewed changes

bleskes added 2 commits December 23, 2016 16:17

Merge remote-tracking branch 'upstream/master' into console_scripts

036b0f6

review feedback

361429e

linting

412633d

epixa added v5.2.0 v6.0.0 labels Dec 23, 2016

epixa merged commit 5616a15 into elastic:master Dec 23, 2016

elastic-jasper mentioned this pull request Dec 23, 2016

[5.x] Make Console Script Aware #9625

Merged

bleskes deleted the console_scripts branch December 23, 2016 19:16

tbragin added the Feature:Dev Tools label Feb 8, 2017

timroes mentioned this pull request Dec 15, 2017

Make triple quoting output in console configurable #15628

Closed

marshallmain mentioned this pull request Apr 15, 2024

[Security Solution] Exceptions don't work on keyword values containing a new line symbol #180836

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Console Script Aware #9433

Make Console Script Aware #9433

bleskes commented Dec 9, 2016

bleskes commented Dec 9, 2016

nik9000 commented Dec 9, 2016

nik9000 commented Dec 9, 2016

nik9000 commented Dec 9, 2016

jdconrad commented Dec 9, 2016

nik9000 commented Dec 9, 2016

epixa Dec 21, 2016

bleskes Dec 23, 2016

epixa Dec 21, 2016

bleskes Dec 23, 2016

epixa Dec 21, 2016

bleskes Dec 23, 2016

epixa Dec 21, 2016

bleskes Dec 23, 2016

bleskes Dec 23, 2016

epixa Dec 21, 2016

epixa Dec 21, 2016

bleskes Dec 23, 2016

epixa Dec 21, 2016 •

edited

bleskes Dec 23, 2016

epixa Dec 21, 2016

bleskes Dec 23, 2016

epixa commented Dec 21, 2016

bleskes commented Dec 23, 2016

epixa commented Dec 23, 2016

epixa commented Dec 23, 2016

epixa commented Dec 23, 2016 •

edited

epixa commented Dec 23, 2016

bleskes commented Dec 23, 2016

epixa commented Dec 23, 2016

bleskes commented Dec 23, 2016

epixa commented Dec 23, 2016


		}).call(ScriptMode.prototype);

		module.exports.ScriptMode = ScriptMode;

Make Console Script Aware #9433

Make Console Script Aware #9433

Conversation

bleskes commented Dec 9, 2016

Script literal:

String literals as fields in documents:

bleskes commented Dec 9, 2016

nik9000 commented Dec 9, 2016

nik9000 commented Dec 9, 2016

nik9000 commented Dec 9, 2016

jdconrad commented Dec 9, 2016

nik9000 commented Dec 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epixa Dec 21, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epixa commented Dec 21, 2016

bleskes commented Dec 23, 2016

epixa commented Dec 23, 2016

epixa commented Dec 23, 2016

epixa commented Dec 23, 2016 • edited

epixa commented Dec 23, 2016

bleskes commented Dec 23, 2016

epixa commented Dec 23, 2016

bleskes commented Dec 23, 2016

epixa commented Dec 23, 2016

epixa Dec 21, 2016 •

edited

epixa commented Dec 23, 2016 •

edited