[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2542 Sure @Leemoonsoo ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2542 Thanks @malayhm. Looks good and i'm merging into master. ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2542 @Leemoonsoo I have added the setting for r and python sql as well. ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2542 Probably i missed. Do you mind add a commit in this branch to enable tab completion on R? ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2542 @Leemoonsoo one question, why did we not add R to have auto complete on tab? ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2542 Thanks @malayhm ! LGTM and merge to master if no further comment! ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2542 @Leemoonsoo Fixed lint error. ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2542 Could you try ``` cd zeppelin-web npm run lint:once ``` and resolve error? ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2542 @zjffdu I have restarted `https://travis-ci.org/malayhm/zeppelin/builds/288324042`, but it seems it didn't help. ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/2542 @malayhm Could you try to return these failed CI build ? https://travis-ci.org/malayhm/zeppelin/builds/288324042 ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2542 @Leemoonsoo @zjffdu I tried merging the latest master with a rerun of the CI job. Unfortunately, it didn't help. Could you suggest the next step? ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/2542 ping @malayhm ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2542 Looks great! There're 3 CI build job failure. https://travis-ci.org/malayhm/zeppelin/builds/286024284 Failures does not look like related to this PR change, but just in case, could you try merge current master (which fixes some CI issue) and see if it gives different result? ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user malayhm commented on the issue:
https://github.com/apache/zeppelin/pull/2542
@Leemoonsoo : I have changed
`let isAllTabs = currentLine.split('').every(function(char) { return (char
=== '\t' || char === ' ') })`
to
`let isAllTabs = currentLine.substring(0, iCursor.column -
1).split('').every(function(char) { return (char === '\t' || char === ' ') })`
---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/2542 ping @malayhm ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2542 I tested this branch + malayhm#1 + master. Like @zjffdu mentioned master branch doesn't have interpreter setting problem, so it works well without ZEPPELIN-2907. @malayhm if you review and merge malayhm#1, i think we can merge this pull request. ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2542 I'm testing this branch + malayhm#1 + master. Let me comment after the test. ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/2542 ping @malayhm @Leemoonsoo ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/2542 > This makes interpreter impossible to add / update "editor" configuration without user recreate interpreterSetting @Leemoonsoo ZEPPELIN-2907 is not resolved, but the issue you concerned above is fixed. As in the interpreter component refactoring, I would always overwrite editor by using the editor info from InterpreterSettingTemplate (interpreter-setting.json) (https://github.com/apache/zeppelin/blob/master/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java#L199). And I believe the editor info is immutable, so it should be safe to overwrite it. That means we could always add new field in editor of interpreter-setting.json ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2542 @malayhm Let me try summarize in this way a. Merge this PR -> Tab completion works but target interpreter to apply tab completion is hardcoded in the front-end. b. Merge this PR + https://github.com/malayhm/zeppelin/pull/1 -> Remove hardcoded part from front-end and let each interpreter choose completion key. But it does not applied to existing interpreter setting [ZEPPELIN-2907](https://issues.apache.org/jira/browse/ZEPPELIN-2907). c. Merge this PR + https://github.com/malayhm/zeppelin/pull/1 + [ZEPPELIN-2907](https://issues.apache.org/jira/browse/ZEPPELIN-2907) -> Everything is happy a) brings something on development of interpreter (i.e. we should document where interpreter developer can find and change completion key from the front-end code). And b) brings something on using zeppelin (i.e. we should document the behavior that existing interpreter setting does not have tab completion key support). If we agree that c) is something we want to deliver to our users, which part do you want to cover in this PR @malayhm ? And according to comment from @zjffdu > @Leemoonsoo I hit the interpreter.json upgrade issue when doing interpreter component refactoring, and I think I already fixed it. https://github.com/apache/zeppelin/blob/master/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java#L199 issue [ZEPPELIN-2907](https://issues.apache.org/jira/browse/ZEPPELIN-2907) is already solved? ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2542 @Leemoonsoo I think the PR branch to this PR has an open discussion and it may break few things. Can we merge this branch to master and take it separately whenever time permits? cc: @zjffdu ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/2542 ping @malayhm @Leemoonsoo Any update ? ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2542 @malayhm just created a PR to this branch https://github.com/malayhm/zeppelin/pull/1. Please take a look. ---
[GitHub] zeppelin issue #2542: ZEPPELIN-277 Add Tab as Autocomplete for Notebook non-...
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2542 @Leemoonsoo Added the improvement as per point 1 mentioned above. It will keep adding tab if all the previous characters are a tab in the same line. ---
