Skip to content
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

[ZEPPELIN-958] Support syntax highlight for python and r interpreter #966

Closed
wants to merge 1 commit into from

Conversation

minahlee
Copy link
Member

@minahlee minahlee commented Jun 6, 2016

What is this PR for?

Support syntax highlight for python and r interpreter

What type of PR is it?

Bug Fix

What is the Jira issue?

ZEPPELIN-958

Screenshots (if appropriate)

Before
screen shot 2016-06-06 at 12 21 56 am

After
screen shot 2016-06-05 at 7 42 55 pm

Before
screen shot 2016-06-05 at 7 30 53 pm

After
screen shot 2016-06-05 at 7 30 31 pm

Questions:

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

@felixcheung
Copy link
Member

LGTM
btw, this is getting fragile - would be great if interpreter can have an extension mechanism to control what it is mapping to on the js side.

@jongyoul
Copy link
Member

jongyoul commented Jun 7, 2016

@felixcheung Could you please tell me your idea in details? Basically I agree with you, but I'm not good at frontend, I don't know whether the feature can be adopted dynamically or not. If it's possible, you could that feature into Interpreter scope. It seems nice.

@jongyoul
Copy link
Member

jongyoul commented Jun 7, 2016

BTW, this PR looks good to me. We can discuss @felixcheung's issue after merging this PR.

@bzz
Copy link
Member

bzz commented Jun 9, 2016

Looks great to me, let's merge as a short term solution.

And yes, as @felixcheung mentioned - in a mid\long term, it would be great for interpreter to encapsulate this knowledge!

@bzz
Copy link
Member

bzz commented Jun 9, 2016

Merging if there is no further discussion

@asfgit asfgit closed this in 19e8ed9 Jun 10, 2016
@felixcheung
Copy link
Member

felixcheung commented Jun 10, 2016

right, I think that can be added to interpreterSettings and checked on the frontend similar to this:

var setting = $scope.interpreterSettings[index];

@bzz
Copy link
Member

bzz commented Jun 16, 2016

Yes.. and this method does not work if the default interpreter is set to Python for the notebook
screen shot 2016-06-17 at 01 03 50

@felixcheung
Copy link
Member

That was only to map the list of interpreter to the language though.

To figure out which interpreter is active in the interpreter right now we have regex to match "%something" at the beginning which won't work for the default interpreter at all.

@minahlee minahlee deleted the ZEPPELIN-958 branch July 7, 2016 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants