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

Support execute-then-quit as arguments in addition to @/path/to/file #189

Closed

Conversation

ericbottard
Copy link
Member

@ericbottard ericbottard commented Dec 24, 2017

Make ResultHandlers configuration more explicit
Handle exit as a dedicated case (prevents eg 'exit' commands in scripts to make script quit)
Add an example of custom ApplicationRunner

Fixes #187
Fixes #183

Handle exit as a dedicated case (prevents eg 'exit' commands in scripts to make script quit)
Add an example of custom ApplicationRunner

Fixes spring-projects#187
Fixes spring-projects#183
@codecov-io
Copy link

codecov-io commented Dec 24, 2017

Codecov Report

Merging #189 into master will decrease coverage by 0.66%.
The diff coverage is 4.25%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #189      +/-   ##
============================================
- Coverage     53.53%   52.86%   -0.67%     
  Complexity      291      291              
============================================
  Files            52       52              
  Lines          1429     1447      +18     
  Branches        223      228       +5     
============================================
  Hits            765      765              
- Misses          603      619      +16     
- Partials         61       63       +2
Impacted Files Coverage Δ Complexity Δ
...framework/shell/standard/CommandValueProvider.java 100% <ø> (ø) 2 <0> (ø) ⬇️
...ll/result/AttributedCharSequenceResultHandler.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...ult/ParameterValidationExceptionResultHandler.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...gframework/shell/SpringShellAutoConfiguration.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...k/shell/standard/StandardAPIAutoConfiguration.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...ework/shell/result/TypeHierarchyResultHandler.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...ework/shell/jline/JLineShellAutoConfiguration.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...ngframework/shell/result/DefaultResultHandler.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...shell/jline/InteractiveShellApplicationRunner.java 0% <0%> (ø) 0 <0> (?)
...framework/shell/result/ThrowableResultHandler.java 0% <0%> (ø) 0 <0> (ø) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e6154a...5a2f0a2. Read the comment docs.

@ericbottard
Copy link
Member Author

Merry Christmas @jvalkeal
Have a look at this PR, I think it could get you what you want to achieve even without allowing override of specific ResultHandlers. Try:

java -jar ./spring-shell-samples/target/spring-shell-samples-2.0.0.BUILD-SNAPSHOT.jar help

and

java -jar ./spring-shell-samples/target/spring-shell-samples-2.0.0.BUILD-SNAPSHOT.jar fail --element-type TYPE

@jvalkeal
Copy link
Contributor

jvalkeal commented Jan 2, 2018

This seem to have a side-effect for exit not exiting from interactive mode properly. Actually you need to exit twice.

Current master:

$ java -jar ./spring-shell-samples/target/spring-shell-samples-2.0.0.BUILD-SNAPSHOT.jar
my-shell:>exit
$

This PR:

$ java -jar ./spring-shell-samples/target/spring-shell-samples-2.0.0.BUILD-SNAPSHOT.jar
my-shell:>exit
shell:>exit
$

@ericbottard
Copy link
Member Author

This is weird. I don't get the behavior you describe, but I do get something odd:
my prompt is not being customized to my-shell:>, while it should

@jvalkeal
Copy link
Contributor

It now works slightly better but looks like you get into trouble with options:

✔ ~/repos/spring/spring-shell [ericbottard-refactor-result-handlers L|✔] 
11:14 $ java -jar ./spring-shell-samples/target/spring-shell-samples-2.0.0.BUILD-SNAPSHOT.jar

  .   ____          _            __ _ _
 /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
 \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
  '  |____| .__|_| |_|_| |_\__, | / / / /
 =========|_|==============|___/=/_/_/_/
 :: Spring Boot ::        (v1.5.8.RELEASE)

my-shell:>sum --v1 1 --v2 2
3
my-shell:>exit
✔ ~/repos/spring/spring-shell [ericbottard-refactor-result-handlers L|✔] 
11:14 $ java -jar ./spring-shell-samples/target/spring-shell-samples-2.0.0.BUILD-SNAPSHOT.jar sum --v1 1 --v2 2

  .   ____          _            __ _ _
 /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
 \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
  '  |____| .__|_| |_|_| |_\__, | / / / /
 =========|_|==============|___/=/_/_/_/
 :: Spring Boot ::        (v1.5.8.RELEASE)

Anonymous option has already been set
✘-1 ~/repos/spring/spring-shell [ericbottard-refactor-result-handlers L|✔] 

I'm not sure if this should now work OOB or if arguments needs some sort of a custom handling.

@ericbottard
Copy link
Member Author

This is because I used an ApplicationRunner and happened to test with the one command that doesn't use --foo (help -C <command>).

I changed the example to use a CommandLineRunner, which you'll need to do as well in skipper. Obviously, if you want to handle some --foo parameters on the command line, you'll need to resolve the ambiguity.
Another approach (but with added quoting dilemmas) is to parse the command(s) you want to execute-then-quit like this:

skipper-shell 'command --arg1 value1 --arg2 value2' 'another-command --arg value'

@ericbottard ericbottard changed the title Make ResultHandlers configuration more explicit Handle exit as a dedicated case (prevents eg 'exit' commands in scripts to make script quit) Add an example of custom ApplicationRunner Support execute-then-quit as arguments in addition to @/path/to/file Jan 15, 2018
@ericbottard
Copy link
Member Author

Merged as f90edd4

@jeremymailen
Copy link

jeremymailen commented Mar 4, 2018

Hi Eric,

Any reason not to include a CommandLineApplicationRunner as standard instead of just the example? I can't think of too many downsides since the shellapp @some/file/path can simultaneously be supported.

In any case spring-shell 1.x had that behavior and we use it quite a bit for automation.

@ericbottard
Copy link
Member Author

The principal reason is that Spring Shell 2 applications are typically Spring Boot applications, and as such may certainly use the --foo.bar=x mechanism to configure their environment. So this would collide and provide unexpected results

@ccaspanello
Copy link

@ericbottard - Can you give me a few details on how the "non-interactive" mode functionality should work.

Sample code works great in interactive mode;

k2:>add 1 2
add(a: 1, b: 2)    <--- from LOG.info() line
3                         <--- from return statement of a+b

However, after I add spring.shell.interactive.enabled=false to application.properties the console exits right away. It doesn't look like the command is running. I tried running the help command with the syntax mentioned above and that didn't even work. Any help would be greatful. FYI - I did clone the repo and build the code to make sure I had the latest and greatest.

@ericbottard
Copy link
Member Author

@ccaspanello The feature described here is not available "out-of-the-box". The setting you mention is supporting it, but additional setup is required. Have a look at ExampleApplicationRunnerConfiguration

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.

None yet

5 participants