Skip to content

Conversation

@astroshim
Copy link
Contributor

What is this PR for?

The code editor tab size is fixed as 4.
It would be better to support changing tabsize to users.

What type of PR is it?

Improvement

Todos

  • - Suppport REST API.

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-887

How should this be tested?

  1. change the tab size in the righ-menu on paragraph.
  2. type tab on paragraph.

Screenshots (if appropriate)

image

Questions:

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

@Leemoonsoo
Copy link
Member

Thanks for addressing the issue. I think it'll be more useful if tab size can be configured globally or per interpreter, rater than individual paragraph. What do you think?

@astroshim
Copy link
Contributor Author

@Leemoonsoo Good point. I agree with you that changing tab size in the interpreter or notebook is better than paragraph. so I think Configuring tab size of notebook level makes more sense than interpreter level, what do you think?

@Leemoonsoo
Copy link
Member

I think tab size is more like per interpreter rather than notebook or paragraph.

Because
appropriate tabsize might different from each interpreter. e.g. 2 for scala, 4 for sparksql and they can be used in the same notebook.

Does it make sense?

@astroshim
Copy link
Contributor Author

astroshim commented May 27, 2016

@Leemoonsoo It really makes sense and I thought that too.
The reason why I thought about configuring tab size in notebook is better was just because easier for the users to set up.
but your example might more make sense.
Thanks for making it clear.

@corneadoug
Copy link
Contributor

@astroshim Would you then create a new PR, or modify this one?

@astroshim
Copy link
Contributor Author

@corneadoug Thank you for your concern! I'm thinking to find right way how to handle this issue.

@bzz
Copy link
Member

bzz commented Jun 21, 2016

To be able to change indention is very reasonable option, esp. as soon as you start working with Python.

@astroshim It could be hard to implement per-interpreter as @Leemoonsoo suggests for now, so how about changing this implementat to per-notebook settings? I think it would be useful.

My only concern is whether it actually changes the paragraph code, or not.

I have checked, and Ace editor we use in soft-tab configuration (although I can not find useSoftTabs anywhere), but the user-input \t gets replaced by 4 spaces now.

Then, how does changing this setting affect the paragraph (that already has no \t)? Will it re-format whole paragraph by adding more whitespace chars (affects the diff)?

@astroshim
Copy link
Contributor Author

@bzz Thank you for your detail review!
I was worring about changing tab size per interpreter as @Leemoonsoo suggests is more hard to config than per notebook or per paragraph.
How about implementing per notebook first and then let's talk it's good or bad?
and This changing is not affect to paragraphs of existing.

@Leemoonsoo
Copy link
Member

Like comment #966 (comment) from @felixcheung, it'll be great if interpreter-setting.json can configure code-editor.

For example, add "editor" field,

[
  {
    "group": "jdbc",
    "name": "sql",
    "className": "org.apache.zeppelin.jdbc.JDBCInterpreter",
    "properties": {
      ...
    },
    "editor" : {
      "tabSize" : 4,
      "syntaxHighlighting" : "sql",
      "autoCompletionOnTab" : true,
      "tabToWhitespace" : true,
      ...
    },

I think this approach can be used not only for 'tabsize', but also for many other configuration values as example above.

Implementing notebook level first would be an option, but if it is not very urgent issue, then it's worth to consider configure through interpreter-setting.json instead. Then many other features (syntaxHighlighting, autoCompletionOnTab, etc) can easily benefit by it.

@astroshim
Copy link
Contributor Author

Thank you for your detailed explanation @Leemoonsoo and I agree with moving interpreter-setting.json.

@astroshim
Copy link
Contributor Author

#1218 is better way.

@astroshim astroshim closed this Jul 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants