Skip to content

Incorrect implementation of non-interactive shells #1218

@fmbenhassine

Description

@fmbenhassine

Currently, non-interactive ShellRunner implementations (non-interactive and script) use the Shell class, which is designed for interactive shells (as it provides a REPL). However, these non interactive implementations rely on an enum that is checked inside the delegate (ie the Shell class) to decide what to do and use exceptions to decide the normal execution flow (which is to exit, see handlingResultNonInt and processExceptionNonInt). Note how ExitRequest is designed to be an exception and is documented as "This exception, when thrown and caught, will ask the shell to gracefully shutdown". It does not make sense to gracefully exit the app with an exception.

I could not find the context of such a decision, but exceptions should only be used for exceptional conditions, but never for ordinary control flows (Effective Java, Item 69). Non interactive shell implementations should not use a REPL:

  • NonInteractiveShellRunner should execute the single command and exit
  • ScriptShellRunner should execute commands defined in the given script and exit
  • InteractiveShellRunner should, however, run the REPL to execute commands interactively until the user requests to exit

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions