Move CLI to use aesh-readline. #1885
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Windows Build 3995 outcome was FAILURE using a merge of a8f29e4 |
|
Linux Build 4672 outcome was FAILURE using a merge of a8f29e4 Failed tests |
|
Windows Build 3996 outcome was FAILURE using a merge of a8f29e4 |
|
Windows Build 3997 outcome was FAILURE using a merge of a8f29e4 Failed tests |
bstansberry
requested changes
Oct 28, 2016
| @@ -41,5 +41,6 @@ public QuitHandler(String command) { | |||
| @Override | |||
| protected void doHandle(CommandContext ctx) { | |||
| ctx.terminateSession(); | |||
| System.exit(0); | |||
There was a problem hiding this comment.
Calling System.exit needs to be carefully controlled or it will break embedded usage. "Quitting" an embedded CLI may not mean the user wants the embedding app to exit. On the server side we do this by using a SystemExiter interface, and all "exit" calls use that. We then swap in different impls of the interface depending on how the server is being used. The standard impl that is used when we know the server was started from the command line calls System.exit. Others do not.
There was a problem hiding this comment.
I was thinking to move the System.exit to the CommandContext. We have all the information needed there to take a decision.
BTW, the current CLI behavior is to exit the process even when a server is embedded.
|
@bstansberry, actually this call to System.exit was not needed. I have removed it. When the aesh console is closed, it exists its waiting loop. The Main thread continues and the CLI exists. |
|
Linux Build 4683 outcome was FAILURE using a merge of 545ea76 Failed tests |
|
Windows Build 4006 outcome was FAILURE using a merge of 545ea76 Failed tests |
|
Linux Build 4695 outcome was FAILURE using a merge of 266f2aa Failed tests |
|
Windows Build 4016 outcome was FAILURE using a merge of 266f2aa Failed tests |
|
Core - Full Integration Build 3652 outcome was FAILURE using a merge of 266f2aa Failed tests |
|
Linux Build 4701 outcome was FAILURE using a merge of 1ca5973 Failed tests |
|
Windows Build 4022 outcome was FAILURE using a merge of 1ca5973 Failed tests |
jfdenise
changed the title
|
Linux Build 4707 outcome was FAILURE using a merge of 579989a Failed tests |
|
Windows Build 4027 outcome was FAILURE using a merge of 579989a Failed tests |
|
Linux Build 4709 outcome was FAILURE using a merge of 579989a |
|
I put the Hold label on this to stop me looking at it while it awaits the move away from requiring the -SNAPSHOT. When it's ready please have Alexey have a look and then ping me. |
|
Core - Full Integration Build 3677 outcome was FAILURE using a merge of f0abd68 |
|
Windows Build 4041 outcome was FAILURE using a merge of f0abd68 Failed tests |
|
Windows Build 4042 outcome was FAILURE using a merge of 8a3e184 Failed tests |
|
retest this please! |
jfdenise
force-pushed
the
readline
branch
3 times, most recently
from
2811379
to
48a3ac5
Compare
|
Full integration - Windows Build 3008 outcome was FAILURE using a merge of 363137e |
|
Core - Full Integration Build 4581 outcome was FAILURE using a merge of 363137e Failed tests |
|
Full integration - Windows Build 3011 outcome was FAILURE using a merge of e72d6e7 |
jfdenise
force-pushed
the
readline
branch
2 times, most recently
from
88edcb4
to
a79c961
Compare
|
Full integration - Windows Build 3114 outcome was FAILURE using a merge of fee0c3c Failed tests |
|
Full integration - Windows Build 3198 outcome was FAILURE using a merge of 4a62587 |
|
Core - Full Integration Build 4771 outcome was FAILURE using a merge of 4a62587 |
|
Core - Full Integration Build 5194 outcome was FAILURE using a merge of 3a42a8a |
|
Full integration - Windows Build 3603 outcome was FAILURE using a merge of 3a42a8a |
|
Core - Full Integration Build 5196 outcome was FAILURE using a merge of 92b1398 |
|
retest this please! |
|
Core - Full Integration Build 5199 outcome was FAILURE using a merge of aac6259 |
|
Core - Full Integration Build 5202 outcome was UNKNOWN using a merge of aac6259 Failed tests |
|
Core - Full Integration Build 5203 outcome was FAILURE using a merge of aac6259 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a a preliminary PR based on ash-readline 1.0-SNAPSHOT.