refactor: make exit command a regular, configurable command by Elec332 · Pull Request #1870 · jline/jline3 (original) (raw)

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

Use the checkboxes below for quick actions:

📝 Walkthrough

Walkthrough

Replace hardcoded REPL exit checks with a pluggable ExitCommands group (configurable via ShellBuilder), centralize EOF handling in the dispatcher, and refactor tests to use shared terminal fixtures and a TestEchoCommand.

Changes

Pluggable Exit Command Feature

Layer / File(s) Summary
Shell Loop Refactor shell/src/main/java/org/jline/shell/Shell.java Reorganized read/dispatch iteration: null handling, trim/empty-line skipping, and dispatch occur in the same try block; removed hardcoded "exit"/"quit" checks.
Exit Command Implementation shell/src/main/java/org/jline/shell/impl/ExitCommands.java New ExitCommands group registers exit and alias quit; execute() throws EndOfFileException.
Builder Configuration shell/src/main/java/org/jline/shell/ShellBuilder.java Adds enableExitCommands field (default true) and public exitCommands(boolean enable) method; build() conditionally registers ExitCommands when enabled.
Dispatcher EOF Handling shell/src/main/java/org/jline/shell/impl/DefaultCommandDispatcher.java Introduces runCommandStage(...) to centralize stage execution, imports EndOfFileException, sets session exit codes, and applies conditional suppression/rethrow logic; executeSingleStage/executePipeGroup now delegate to it.
Test Terminal Base shell/src/test/java/org/jline/shell/AbstractTerminalTest.java Adds AbstractTerminalTest that wires piped I/O streams, builds a dumb(true) Terminal in setUp(), and closes resources in tearDown().
Test Base Refactor shell/src/test/java/org/jline/shell/impl/AbstractCommandDispatcherTest.java, shell/src/test/java/org/jline/shell/impl/AbstractCommandsTest.java Tests now extend AbstractTerminalTest; removed constructor-based dispatcher factories; added createDispatcher() hook; tearDown() delegates terminal cleanup.
TestEchoCommand & Fixture shell/src/test/java/org/jline/shell/impl/TestEchoCommand.java Adds TestEchoCommand fixture (constructor, description, execute) used by multiple tests.
ShellBuilder Tests shell/src/test/java/org/jline/shell/ShellBuilderTest.java Refactors to use shared terminal base and adds tests for prompt, reader options, history, command groups, onReaderReady, and exit enable/disable behavior.
Default Dispatcher Tests shell/src/test/java/org/jline/shell/impl/DefaultCommandDispatcherTest.java Adds ExitCommand fixture throwing EndOfFileException; adds integration tests covering exit in pipeline/AND/OR/sequence contexts; updates setups to use TestEchoCommand.
Misc Test Adjustments shell/src/test/java/org/jline/shell/impl/* Help/IORedirection/SignalHandling/CommandHighlighter/SimpleCommandGroup/VariableCommands tests updated to use shared fixtures and remove inline command classes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

enhancement

Suggested reviewers

Poem

🐰 I hopped through code and trimmed each line,
Exit is a command now, tidy and fine.
Tests share pipes and echoes all around,
Dispatcher greets EOF and sets the code sound,
A cheerful rabbit dances on the shell's new ground.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor: make exit command a regular, configurable command' directly and accurately summarizes the main change: converting the exit command from special built-in handling to a standard, optional, configurable command.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches 🧪 Generate unit tests (beta)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.