Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

TAJO-2064: Supporting auto-completion in Tsql #955

Closed
wants to merge 10 commits into from

Conversation

eminency
Copy link
Contributor

@eminency eminency commented Feb 1, 2016

Use tab key in Tsql

@blrunner
Copy link
Contributor

Sorry for late review. I'll finish to review this PR in a few days.

@eminency
Copy link
Contributor Author

Never mind.
It is not a critical patch.

@blrunner
Copy link
Contributor

blrunner commented Mar 7, 2016

@eminency

If you implement Completer to DescFunctionCommand that shows built-in function description, this patch will be more useful.

@eminency
Copy link
Contributor Author

eminency commented Mar 7, 2016

@blrunner

Good, I've done.

}

private Collection<String> getKeywords() {
List<String> klist = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like a ambiguous name. How about rename it to keywords or keywordList?

}
}

class ConfCompleter extends StringsCompleter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, many tajo inner-classes have been declared inside their top-class. How about put ConfCompleter inside TajoGetConfCommand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is a good suggestion.

@eminency
Copy link
Contributor Author

eminency commented Mar 8, 2016

@blrunner
Ok, please check what you commented.

@blrunner
Copy link
Contributor

blrunner commented Mar 8, 2016

@eminency

Thanks for your reflection.

Well, I have one more suggestion. Currently, Tajo provides a SQL grammar file as following:

https://github.com/apache/tajo/blob/master/tajo-sql-parser/src/main/antlr4/org/apache/tajo/parser/sql/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.

@eminency
Copy link
Contributor Author

eminency commented Mar 9, 2016

@blrunner
SQL keywords are fetched from SQLLexer now.

@blrunner
Copy link
Contributor

+1

Sorry for late review.
I found that the auto-completion ran as expected and your reflection about my opinion.

@asfgit asfgit closed this in 17e61bd Mar 18, 2016
@eminency eminency deleted the auto-completion branch April 22, 2016 01:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants