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

minahlee · 2016-09-08T15:00:45Z

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

How should this be tested?

Create new note and make markdown interpreter to be default.
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.

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

Case 2. When use alias name as repl name.

Before

After

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.

When default bound interpreter changes, syntax highlight is not changed accordingly
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)

minahlee · 2016-09-12T13:00:45Z

CI is green, ready for review.

bzz · 2016-09-13T06:37:21Z

docs/development/writingzeppelininterpreter.md

        "description": "Property 2 description"
      }, ...
+    },
+    "editor": {


Quick question - it's a bit not clear if this section is mandatory and if not, what is going to be the default one? May be it could be described in docs below.
Asking as there are i.e PRs that implement a new interpreters, before this section was introduced.

Good point! If the language is not specified plain text mode will be used for syntax highlight, which will highlight nothing. I added description in fd7896e. Thanks for review :)

bzz · 2016-09-13T06:40:19Z

Looks great to me.

Leemoonsoo · 2016-09-13T20:15:32Z

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java

+    return factory.getInterpreter(getId(), name);
+  }
+
+  public Map<String, Object> getEditorSetting(String replName) {


I think this method is more make sense to be placed in InterpreterFactory, while editor setting is related interpreter and interpreter setting, not each note. What do you think?

@Leemoonsoo It totally makes sense, I made change in e810da4. Please review.

Leemoonsoo · 2016-09-21T21:54:37Z

@minahlee Great work! LGTM

…json after apache#1145

…ont-end <-> back-end communication

minahlee · 2016-09-22T15:04:03Z

Merge if there is no more discussion

### 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

minahlee changed the title ~~[ZEPPELIN-1026] set syntax highlight based on default bound interpreter~~ [WIP][ZEPPELIN-1026] set syntax highlight based on default bound interpreter Sep 9, 2016

minahlee force-pushed the ZEPPELIN-1026 branch from 9bc9e88 to d50ba38 Compare September 12, 2016 12:13

minahlee changed the title ~~[WIP][ZEPPELIN-1026] set syntax highlight based on default bound interpreter~~ [ZEPPELIN-1026] set syntax highlight based on default bound interpreter Sep 12, 2016

bzz reviewed Sep 13, 2016
View reviewed changes

Leemoonsoo reviewed Sep 13, 2016
View reviewed changes

felixcheung mentioned this pull request Sep 17, 2016

[ZEPPELIN-682] New interpreter for Apache Beam (incubating)/DataFlow #1334

Closed

minahlee force-pushed the ZEPPELIN-1026 branch 4 times, most recently from 69fc151 to e810da4 Compare September 21, 2016 08:39

minahlee force-pushed the ZEPPELIN-1026 branch 3 times, most recently from ed2f473 to aa5eb2f Compare September 22, 2016 09:53

minahlee added 10 commits September 22, 2016 16:25

Add editor field

26cbbb8

Align comments for readability

52f4207

Get paragraph editor mode from backend

20132ca

[DOC] Setting syntax highlight when writing new interpreter

565d9d0

Add test

75543b3

Change the way to read interpreter language from interpreter-setting.…

9e4f2e9

…json after apache#1145

Change condition for triggering 'getAndSetEditorSetting' to reduce fr…

0874522

…ont-end <-> back-end communication

Fix test

08ccad9

Add description about default syntax highlight in doc

2d56222

Move getEditorSetting to InterpreterFactory class

c66fb0e

minahlee force-pushed the ZEPPELIN-1026 branch from aa5eb2f to c66fb0e Compare September 22, 2016 14:25

asfgit closed this in a3ca800 Sep 23, 2016

minahlee deleted the ZEPPELIN-1026 branch September 24, 2016 19:05

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 #1415

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

Uh oh!

minahlee commented Sep 8, 2016

Uh oh!

minahlee commented Sep 12, 2016

Uh oh!

bzz Sep 13, 2016

Uh oh!

minahlee Sep 13, 2016 •

edited

Loading

Uh oh!

bzz commented Sep 13, 2016

Uh oh!

Leemoonsoo Sep 13, 2016

Uh oh!

minahlee Sep 19, 2016 •

edited

Loading

Uh oh!

Leemoonsoo commented Sep 21, 2016

Uh oh!

minahlee commented Sep 22, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

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

Uh oh!

Conversation

minahlee commented Sep 8, 2016

What is this PR for?

What type of PR is it?

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Case 1. When the default interpreter set to python interpreter.

Case 2. When use alias name as repl name.

Further possible improvements

Questions:

Uh oh!

minahlee commented Sep 12, 2016

Uh oh!

bzz Sep 13, 2016

Choose a reason for hiding this comment

Uh oh!

minahlee Sep 13, 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 commented Sep 13, 2016

Uh oh!

Leemoonsoo Sep 13, 2016

Choose a reason for hiding this comment

Uh oh!

minahlee Sep 19, 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!

Leemoonsoo commented Sep 21, 2016

Uh oh!

minahlee commented Sep 22, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

minahlee Sep 13, 2016 •

edited

Loading

minahlee Sep 19, 2016 •

edited

Loading