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

Migration to Jline3 #115

Closed
wants to merge 6 commits into from
Closed

Migration to Jline3 #115

wants to merge 6 commits into from

Conversation

snuyanzin
Copy link
Collaborator

@snuyanzin snuyanzin commented Aug 22, 2018

The PR is intended to move to jline3, jdk8

What is done

  1. Required jdk -> 1.8+
  2. Required jline -> 3.9.0 (also included jna, jansi artifacts required for Windows)
  3. Multiline mode if quotes are not closed or a line not finished with a semicolon (in case of non-command or !sql, !all). As a hint a missing symbol is mentioned in prompt. And now becomes possible to get the whole multiline command from the history via up/down. Something similar was requested in Re-execute the previous query #73. SqlLineParser and SqlLineParserTest classes.
  4. Usage of internal jline history
  5. Usage of jline3's less for !manual in case of not Windows. In case of Windows usage of old silly pager as current less implementation fails on Windows Less fails On Windows with java.util.regex.PatternSyntaxException jline/jline3#304
  6. Test for !manual is switched off as it required currently not supported interactive mode in tests. (Also some comments below about these last bullets)

Also as a 'side effect' vi-like keymap binding is available now,
I guess something similar was requested in #60
Also there have come many other jline3 features

What is under question

1. About ways to test !manual. Currently I have the next in my mind
- As the test were intended to check if manual.txt present or not (the original issue #49 ) we could do exactly this test
- Test with timeout which could help with exit from interactive mode
2. Jline3 uses timestamps in their history files. The problem could be if a user of jline2-based sqlline will try to use jline3-based. Here there are several ways to resolve the issue

@snuyanzin snuyanzin force-pushed the JLINE3 branch 6 times, most recently from 67e67c8 to 93221f6 Compare August 23, 2018 10:24
This was referenced Sep 2, 2018
@snuyanzin snuyanzin force-pushed the JLINE3 branch 5 times, most recently from 486b4f7 to ee72e9a Compare September 10, 2018 23:25
1. Required jdk -> 1.8+
2. Required jline -> 3.9.0 (also included jna, jansi artifacts required for Windows)
3. Multiline mode if quotes are not closed or a line not finished with a semicolon (in case of non-command or !sql, !all). As a hint a missing symbol is mentioned in prompt. And now becomes possible to get the whole multiline command from the history via up/down. SqlLineParser and SqlLineParserTest classes.
4. Usage of internal jline history
5. Test for !manual is switched off as it required currently not supported interactive mode in tests. (Also some comments below about these last bullets)
@snuyanzin snuyanzin force-pushed the JLINE3 branch 8 times, most recently from 3f59858 to 5f61e22 Compare September 12, 2018 07:38
1. Rebased to sqlline 1.15.0
2. !manual test fixed
@snuyanzin snuyanzin force-pushed the JLINE3 branch 4 times, most recently from f2d06d2 to a99e977 Compare September 12, 2018 10:01
@snuyanzin
Copy link
Collaborator Author

  1. Rebased to sqlline 1.5.0
  2. Finally mocked less to get !manual test working
  3. Added !rerun (also !/ is possible by analogy with Oracle sqlplus) command to execute previous command from history file:

@snuyanzin
Copy link
Collaborator Author

Added padded prompt in a similar way it is before moving to jline3 for instance

0: jdbc:calcite:model=example/csv/target/test> select '
. . . . . . . . . . . . . . . . . . . . quote> 1'
. . . . . . . . . . . . . . . . . . semicolon> ;
+--------+
| EXPR$0 |
+--------+
|
1     |
+--------+
1 row selected (0.598 seconds)

Added additional tests for !rerun, !/ commands based on generated history file
@snuyanzin
Copy link
Collaborator Author

@julianhyde could I ask you to have a look and merge if it looks ok from your side or put some comments otherwise.
This PR allows to use java8 and jline3 features and there are already other issues depending on it e.g. #129.

@julianhyde
Copy link
Owner

Merged as bf495f9; fixes #105, #73, #60.

@julianhyde julianhyde closed this Oct 4, 2018
@julianhyde
Copy link
Owner

@snuyanzin I know you fixes for #154 and #155 in another branch. Can you please rebase on the new master?

@snuyanzin snuyanzin deleted the JLINE3 branch October 24, 2018 13:59
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.

2 participants