[ZEPPELIN-1026] set syntax highlight based on default bound interpreter #1148

minahlee · 2016-07-07T13:51:01Z

What is this PR for?

Set syntax highlight based on default interpreter if %magic is not set
Get syntax highlight mode from backend

What type of PR is it?

Improvement

Todos

Add test
Update writingzeppelininterpreter.md
Fix test
Test after [ZEPPELIN-1012] Support Interpreter Aliases #1145 merged, modify if necessary

What is the Jira issue?

ZEPPELIN-1026

Screenshots (if appropriate)

This is screenshot when the default interpreter set to python interpreter.
Before
Has scala as syntax highlight language when %python is not set.

After
Has python as syntax highlight language even when %python is not set.

Questions:

Does the licenses files need update? no
Is there breaking changes for older versions? no
Does this needs documentation? yes

felixcheung · 2016-07-07T21:57:44Z

zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js

    'ace/mode/sql': /^%(\w*\.)?\wql/,
-    'ace/mode/markdown': /^%md/,
-    'ace/mode/sh': /^%sh/
+    'ace/mode/markdown': /^%md/


should we get rid of these then?

This PR reads syntax highlight language from interpreter-setting.json introduced by #835. I didn't remove this line because %md, %*ql is not following this new interpreter registration mechanism at the moment, in other words %md, %*ql interpreter doesn't have interpreter-setting.json file.
You can see the list of interpretesr that can apply new syntax highlight mechanism in ecb0af2

Not sure how this would work with jdbc's alias magic style too, eg. %jdbc(hive), or %hive?

cloverhearts · 2016-07-08T07:26:59Z

flink/src/main/resources/interpreter-setting.json

      }
+    },
+    "editor": {
+      "mode": "scala"


In my personal opinion,
"Mode" is not a bad name. But better "language" right.
mode stands for 'launguage mode' of ui-ace.
However, Interpreter shall not directly related to the ui-ace.
In addition, its value is "kind of language."
I believe that the future of Zeppelin may not use the ace editor.
If so, the name "mode" can cause confusion.
What do you think?

Valid point indeed. let me update it

felixcheung · 2016-07-09T06:05:42Z

zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js

+        if (!paragraphText.startsWith('%')) {
+          magic = $scope.$parent.interpreterBindings[0].group;
+        } else {
+          var replNameRegexp = /%(.+?)\s/g;


This seems to capture the whitespace after any character as well, eg.
it matches %1 for capture group 1 - notice the space after 1.

bzz · 2016-07-11T02:43:46Z

zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java

            unicastUpdateNotebookJobInfo(conn, messagereceived);
            break;
+          case EDITOR_SETTING:
+            getEditorSetting(conn, notebook, messagereceived);


As far as I understood we introduce this new message type to current WS api in order just to pass this information to the front-end, right?

I wonder if there is another, simpler way to do it? As we are not changing it from the front, there is much less value in broadcasting it though WS (that's what we use WS mostly for), so it makes me think that there must be a simpler way to just pass this information to the front..or do I miss something here?

What do you think?

bzz · 2016-08-11T04:32:24Z

@minahlee do you think we should close it for now or rename it to WIP?

minahlee · 2016-08-11T04:39:54Z

I think I will create another PR for this so closing it.

### What is this PR for? This is complete work of #1148. Comments and tasks on #1148 has been handled in this PR. - Add syntax language information in `interpreter-setting.json` - When user type `%replName` in paragraph, back-end check if the interpreter name with `replName` exists, and return language information to front-end if it does - If user doesn't specify `%replName`, default interpreter's language will be used - Using alias name for paragraph syntax highlight ### What type of PR is it? [Bug Fix | Improvement] ### What is the Jira issue? [ZEPPELIN-1026](https://issues.apache.org/jira/browse/ZEPPELIN-1026) ### How should this be tested? 1. Create new note and make markdown interpreter to be default. 2. See if markdown syntax is applied. ### Screenshots (if appropriate) #### Case 1. When the default interpreter set to python interpreter. **Before** Has `scala` as syntax highlight language when %python is not set. <img width="665" alt="screen shot 2016-07-07 at 10 46 20 pm" src="https://cloud.githubusercontent.com/assets/8503346/16655312/af67a302-4494-11e6-949e-793ad0515d7a.png"> **After** Has `python` as syntax highlight language even when %python is not set. <img width="666" alt="screen shot 2016-07-07 at 10 44 39 pm" src="https://cloud.githubusercontent.com/assets/8503346/16655248/769d8ba4-4494-11e6-9b3c-dc5e026e9c53.png"> #### Case 2. When use alias name as repl name. **Before** <img width="742" alt="screen shot 2016-09-08 at 4 22 39 pm" src="https://cloud.githubusercontent.com/assets/8503346/18353471/620c5ede-75e2-11e6-9d01-0726bc900dc0.png"> **After** <img width="741" alt="screen shot 2016-09-08 at 4 34 57 pm" src="https://cloud.githubusercontent.com/assets/8503346/18353487/6cdaa406-75e2-11e6-831a-08e0fa3a85d8.png"> ### Further possible improvements There are still several cases that Zeppelin doesn't handle syntax highlight well. These can be handled with another jira ticket/PR. 1. When default bound interpreter changes, syntax highlight is not changed accordingly 2. When copy/paste code, syntax highlight won't be applied properly since Zeppelin only checks changes when cursor is in first line. ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? yes(for creating new interpreter) Author: Mina Lee <[email protected]> Closes #1415 from minahlee/ZEPPELIN-1026 and squashes the following commits: c66fb0e [Mina Lee] Move getEditorSetting to InterpreterFactory class 2d56222 [Mina Lee] Add description about default syntax highlight in doc 08ccad9 [Mina Lee] Fix test 0874522 [Mina Lee] Change condition for triggering 'getAndSetEditorSetting' to reduce front-end <-> back-end communication 9e4f2e9 [Mina Lee] Change the way to read interpreter language from interpreter-setting.json after #1145 75543b3 [Mina Lee] Add test 565d9d0 [Mina Lee] [DOC] Setting syntax highlight when writing new interpreter 20132ca [Mina Lee] Get paragraph editor mode from backend 52f4207 [Mina Lee] Align comments for readability 26cbbb8 [Mina Lee] Add editor field

### What is this PR for? This is complete work of apache#1148. Comments and tasks on apache#1148 has been handled in this PR. - Add syntax language information in `interpreter-setting.json` - When user type `%replName` in paragraph, back-end check if the interpreter name with `replName` exists, and return language information to front-end if it does - If user doesn't specify `%replName`, default interpreter's language will be used - Using alias name for paragraph syntax highlight ### What type of PR is it? [Bug Fix | Improvement] ### What is the Jira issue? [ZEPPELIN-1026](https://issues.apache.org/jira/browse/ZEPPELIN-1026) ### How should this be tested? 1. Create new note and make markdown interpreter to be default. 2. See if markdown syntax is applied. ### Screenshots (if appropriate) #### Case 1. When the default interpreter set to python interpreter. **Before** Has `scala` as syntax highlight language when %python is not set. <img width="665" alt="screen shot 2016-07-07 at 10 46 20 pm" src="https://cloud.githubusercontent.com/assets/8503346/16655312/af67a302-4494-11e6-949e-793ad0515d7a.png"> **After** Has `python` as syntax highlight language even when %python is not set. <img width="666" alt="screen shot 2016-07-07 at 10 44 39 pm" src="https://cloud.githubusercontent.com/assets/8503346/16655248/769d8ba4-4494-11e6-9b3c-dc5e026e9c53.png"> #### Case 2. When use alias name as repl name. **Before** <img width="742" alt="screen shot 2016-09-08 at 4 22 39 pm" src="https://cloud.githubusercontent.com/assets/8503346/18353471/620c5ede-75e2-11e6-9d01-0726bc900dc0.png"> **After** <img width="741" alt="screen shot 2016-09-08 at 4 34 57 pm" src="https://cloud.githubusercontent.com/assets/8503346/18353487/6cdaa406-75e2-11e6-831a-08e0fa3a85d8.png"> ### Further possible improvements There are still several cases that Zeppelin doesn't handle syntax highlight well. These can be handled with another jira ticket/PR. 1. When default bound interpreter changes, syntax highlight is not changed accordingly 2. When copy/paste code, syntax highlight won't be applied properly since Zeppelin only checks changes when cursor is in first line. ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? yes(for creating new interpreter) Author: Mina Lee <[email protected]> Closes apache#1415 from minahlee/ZEPPELIN-1026 and squashes the following commits: c66fb0e [Mina Lee] Move getEditorSetting to InterpreterFactory class 2d56222 [Mina Lee] Add description about default syntax highlight in doc 08ccad9 [Mina Lee] Fix test 0874522 [Mina Lee] Change condition for triggering 'getAndSetEditorSetting' to reduce front-end <-> back-end communication 9e4f2e9 [Mina Lee] Change the way to read interpreter language from interpreter-setting.json after apache#1145 75543b3 [Mina Lee] Add test 565d9d0 [Mina Lee] [DOC] Setting syntax highlight when writing new interpreter 20132ca [Mina Lee] Get paragraph editor mode from backend 52f4207 [Mina Lee] Align comments for readability 26cbbb8 [Mina Lee] Add editor field

felixcheung reviewed Jul 7, 2016
View reviewed changes

minahlee closed this Jul 8, 2016

minahlee reopened this Jul 8, 2016

cloverhearts reviewed Jul 8, 2016
View reviewed changes

minahlee force-pushed the ZEPPELIN-1026 branch from 6535dd4 to ba65848 Compare July 9, 2016 03:10

felixcheung reviewed Jul 9, 2016
View reviewed changes

minahlee force-pushed the ZEPPELIN-1026 branch 2 times, most recently from b04d3d9 to 52d7472 Compare July 10, 2016 01:50

bzz reviewed Jul 11, 2016
View reviewed changes

minahlee force-pushed the ZEPPELIN-1026 branch from 52d7472 to 5638f52 Compare July 16, 2016 06:11

minahlee added 7 commits July 19, 2016 16:17

Add editor field

9251f76

Align comments for readability

a5e9db3

Get paragraph editor mode from backend

9788b54

[DOC] Setting syntax highlight when writing new interpreter

7ba631c

Add unit test for editor setting

0a05640

Fix test

fee797f

Rename mode to language

20bc6ed

minahlee force-pushed the ZEPPELIN-1026 branch from 5638f52 to 20bc6ed Compare July 19, 2016 07:56

minahlee closed this Aug 11, 2016

minahlee mentioned this pull request Sep 8, 2016

[ZEPPELIN-1026] set syntax highlight based on default bound interpreter #1415

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ZEPPELIN-1026] set syntax highlight based on default bound interpreter #1148

[ZEPPELIN-1026] set syntax highlight based on default bound interpreter #1148

Uh oh!

minahlee commented Jul 7, 2016 •

edited

Loading

Uh oh!

felixcheung Jul 7, 2016

Uh oh!

minahlee Jul 7, 2016

Uh oh!

felixcheung Jul 9, 2016

Uh oh!

cloverhearts Jul 8, 2016

Uh oh!

minahlee Jul 9, 2016

Uh oh!

felixcheung Jul 9, 2016 •

edited

Loading

Uh oh!

bzz Jul 11, 2016

Uh oh!

bzz commented Aug 11, 2016

Uh oh!

minahlee commented Aug 11, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[ZEPPELIN-1026] set syntax highlight based on default bound interpreter #1148

[ZEPPELIN-1026] set syntax highlight based on default bound interpreter #1148

Uh oh!

Conversation

minahlee commented Jul 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

Screenshots (if appropriate)

Questions:

Uh oh!

felixcheung Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

minahlee Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

felixcheung Jul 9, 2016

Choose a reason for hiding this comment

Uh oh!

cloverhearts Jul 8, 2016

Choose a reason for hiding this comment

Uh oh!

minahlee Jul 9, 2016

Choose a reason for hiding this comment

Uh oh!

felixcheung Jul 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bzz Jul 11, 2016

Choose a reason for hiding this comment

Uh oh!

bzz commented Aug 11, 2016

Uh oh!

minahlee commented Aug 11, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

minahlee commented Jul 7, 2016 •

edited

Loading

felixcheung Jul 9, 2016 •

edited

Loading