You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by eminency <gi...@git.apache.org> on 2016/02/01 06:45:44 UTC
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
GitHub user eminency opened a pull request:
https://github.com/apache/tajo/pull/955
TAJO-2064: Supporting auto-completion in Tsql
Use tab in Tsql!
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/eminency/tajo auto-completion
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/955.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #955
----
commit b7f89ad05b238ef97bfa7b7760454e2f0f64b58b
Author: Jongyoung Park <em...@gmail.com>
Date: 2016-01-26T08:25:45Z
initial version
commit 111add133dbdbd50a0c49a4499fd3159096548dd
Author: Jongyoung Park <em...@gmail.com>
Date: 2016-01-26T09:04:49Z
getconf basic
commit acc63141e5bb9b155db93eb455e303dc9f9e843b
Author: Jongyoung Park <em...@gmail.com>
Date: 2016-01-27T09:23:27Z
getconf completion
commit 077ba1b9d0e59854fdf3fdc54ada242b388dfae3
Author: Jongyoung Park <em...@gmail.com>
Date: 2016-02-01T05:43:32Z
SQL keywords are added
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on the pull request:
https://github.com/apache/tajo/pull/955#issuecomment-183840136
Never mind.
It is not a critical patch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/955#issuecomment-193593546
@eminency
Thanks for your reflection.
Well, I have one more suggestion. Currently, Tajo provides a SQL grammar file as following:
https://github.com/apache/incubator-tajo/blob/master/tajo-core/tajo-core-backend/src/main/antlr4/org/apache/tajo/engine/parser/SQLLexer.g4
But if we have existing grammar file and ``SQLKeywords``, when other contributors add new grammar, they must add it to above two files. I think that it is a little inefficient for maintaing codes. Even though it may be trouble, if you use ``SQLLexer.g4``, this patch will be better. For reference, there are some unnecessary keywords to ``SQLLexter.g4`` on tsql. So you need to filter theme at ``TajoCli::getKeywords``.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/955#issuecomment-193058129
@eminency
If you implement ``Completer`` to ``DescFunctionCommand`` that shows built-in function description, this patch will be more useful.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tajo/pull/955
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/955#issuecomment-183822941
Sorry for late review. I'll finish to review this PR in a few days.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on a diff in the pull request:
https://github.com/apache/tajo/pull/955#discussion_r55177400
--- Diff: tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/TajoGetConfCommand.java ---
@@ -55,4 +62,52 @@ public String getUsage() {
public String getDescription() {
return "execute a tajo getconf command.";
}
+
+ @Override
+ public ArgumentCompleter getArgumentComplementer() {
+ TajoConf.ConfVars[] vars = TajoConf.ConfVars.values();
+ List<String> confNames = new ArrayList<>();
+
+ for(TajoConf.ConfVars varname: vars) {
+ confNames.add(varname.varname);
+ }
+
+ return new ArgumentCompleter(
+ new StringsCompleter(getCommand()),
+ new ConfCompleter(confNames.toArray(new String[confNames.size()])),
+ NullCompleter.INSTANCE);
+ }
+}
+
+class ConfCompleter extends StringsCompleter {
--- End diff --
Currently, many tajo inner-classes have been declared inside their top-class. How about put ``ConfCompleter`` inside ``TajoGetConfCommand``?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/955#discussion_r55176131
--- Diff: tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/TajoShellCommand.java ---
@@ -126,4 +136,56 @@ protected void print(char c, int count) {
println();
return columnWidths;
}
+
+ public ArgumentCompleter getArgumentComplementer() {
--- End diff --
Good catch
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on a diff in the pull request:
https://github.com/apache/tajo/pull/955#discussion_r55175338
--- Diff: tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/TajoShellCommand.java ---
@@ -126,4 +136,56 @@ protected void print(char c, int count) {
println();
return columnWidths;
}
+
+ public ArgumentCompleter getArgumentComplementer() {
--- End diff --
It seems to be a typo.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on the pull request:
https://github.com/apache/tajo/pull/955#issuecomment-193579952
@blrunner
Ok, please check what you commented.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/955#discussion_r55304007
--- Diff: tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/TajoGetConfCommand.java ---
@@ -55,4 +62,52 @@ public String getUsage() {
public String getDescription() {
return "execute a tajo getconf command.";
}
+
+ @Override
+ public ArgumentCompleter getArgumentComplementer() {
+ TajoConf.ConfVars[] vars = TajoConf.ConfVars.values();
+ List<String> confNames = new ArrayList<>();
+
+ for(TajoConf.ConfVars varname: vars) {
+ confNames.add(varname.varname);
+ }
+
+ return new ArgumentCompleter(
+ new StringsCompleter(getCommand()),
+ new ConfCompleter(confNames.toArray(new String[confNames.size()])),
+ NullCompleter.INSTANCE);
+ }
+}
+
+class ConfCompleter extends StringsCompleter {
--- End diff --
I think it is a good suggestion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on the pull request:
https://github.com/apache/tajo/pull/955#issuecomment-194071033
@blrunner
SQL keywords are fetched from SQLLexer now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on the pull request:
https://github.com/apache/tajo/pull/955#issuecomment-193146390
@blrunner
Good, I've done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on a diff in the pull request:
https://github.com/apache/tajo/pull/955#discussion_r55175302
--- Diff: tajo-cli/src/main/java/org/apache/tajo/cli/tsql/TajoCli.java ---
@@ -374,11 +387,43 @@ private void initCommands() {
System.err.println(e.getMessage());
throw new RuntimeException(e.getMessage());
}
+
+ // make completers for console auto-completion
+ compList.add(cmd.getArgumentComplementer());
+
commands.put(cmd.getCommand(), cmd);
for (String alias : cmd.getAliases()) {
commands.put(alias, cmd);
}
}
+
+ cliCompleter = new AggregateCompleter(compList);
+
+ sqlCompleter = new ArgumentCompleter(
+ new ArgumentCompleter.AbstractArgumentDelimiter() {
+ @Override
+ public boolean isDelimiterChar(CharSequence buf, int pos) {
+ char c = buf.charAt(pos);
+ return Character.isWhitespace(c) || !(Character.isLetterOrDigit(c)) && c != '_';
+ }
+ },
+ new StringsCompleter(getKeywords())
+ );
+ }
+
+ private Collection<String> getKeywords() {
+ List<String> klist = new ArrayList<>();
--- End diff --
It looks like a ambiguous name. How about rename it to ``keywords`` or ``keywordList``?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2064: Supporting auto-completion in Tsql
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/955#issuecomment-198165166
+1
Sorry for late review.
I found that the auto-completion ran as expected and your reflection about my opinion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---