feat: Add timeout command to wait for a specified duration #88
Conversation
|
🚀 Hi @nihaltp! Thank you for contributing to MyCMD. A maintainer will review your PR shortly. 🎉 |
|
Warning Rate limit exceeded@nihaltp has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughIntroduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Shell as ShellContext
participant Timeout as TimeoutCommand
participant Pause as PauseCommand
participant Reader as InputReaderDaemon
User->>Timeout: execute(args, context)
Timeout->>Timeout: parse args (/t, /nobreak, bare number)
alt timeout == -1
Timeout->>Pause: delegate execution
Pause->>User: pause behavior
else timeout >= 0
alt /nobreak not set
Timeout->>Reader: start daemon to watch System.in
Reader->>Timeout: signal if Enter pressed
end
loop each second
Timeout->>User: print remaining seconds
end
alt Enter pressed
Reader->>Timeout: interruption signal
Timeout->>User: interrupted message
else completed
Timeout->>User: completion message
end
Timeout->>Timeout: drain remaining input
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
…eature/timeout
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/com/mycmd/commands/TimeoutCommand.java (2)
49-58: Consider extracting the magic number to a constant.The validation logic is correct. However, the maximum timeout value (99999) is hardcoded and appears in the error message as well.
Consider extracting to a constant for maintainability:
+ private static final int MAX_TIMEOUT_SECONDS = 99999; + @Override public void execute(String[] args, ShellContext context) { ... - if (seconds < -1 || seconds > 99999) { + if (seconds < -1 || seconds > MAX_TIMEOUT_SECONDS) { System.out.println( - "Error: Invalid value for timeout specified. Valid range is 0-99999 seconds."); + "Error: Invalid value for timeout specified. Valid range is 0-" + MAX_TIMEOUT_SECONDS + " seconds."); return; }
107-116: Improve the empty catch block comment.The input cleanup logic is necessary, but the empty catch block could have a more descriptive comment explaining why it's safe to ignore the IOException.
} catch (IOException e) { - // Ignore + // Ignore - best effort cleanup, failures don't affect functionality }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/mycmd/App.java(1 hunks)src/main/java/com/mycmd/commands/TimeoutCommand.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/mycmd/commands/TimeoutCommand.java (1)
src/main/java/com/mycmd/ShellContext.java (1)
ShellContext(7-151)
🔇 Additional comments (5)
src/main/java/com/mycmd/App.java (1)
111-111: LGTM!The timeout command registration follows the established pattern and integrates correctly with the command registry.
src/main/java/com/mycmd/commands/TimeoutCommand.java (4)
10-47: LGTM!The argument parsing logic correctly handles all the specified formats (/t , /nobreak, bare numbers) with proper validation for duplicates and missing values. The error messages are clear and match expected command-line behavior.
60-65: LGTM!Correctly delegates to PauseCommand when seconds is -1, matching Windows CMD timeout behavior.
83-106: LGTM!The countdown loop correctly handles both
/nobreakand normal modes, with appropriate interrupt handling and user-friendly in-place countdown updates.
118-129: LGTM!The description and usage documentation clearly convey the command's purpose and all supported syntax variants.
|
🚀 Hi @nihaltp! Thank you for contributing to MyCMD. A maintainer will review your PR shortly. 🎉 |
|
🚀 Hi @nihaltp! Thank you for contributing to MyCMD. A maintainer will review your PR shortly. 🎉 |
…to feature/timeout
|
🚀 Hi @nihaltp! Thank you for contributing to MyCMD. A maintainer will review your PR shortly. 🎉 |
…to feature/timeout
…to feature/timeout
|
🚀 Hi @nihaltp! Thank you for contributing to MyCMD. A maintainer will review your PR shortly. 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/main/java/com/mycmd/commands/TimeoutCommand.java (1)
76-82: Behavior now matches prompt (Enter-only). Nice.You now exit only on Enter and the message reflects that. Good alignment with UX.
Also applies to: 103-109
🧹 Nitpick comments (3)
src/main/java/com/mycmd/commands/TimeoutCommand.java (3)
103-109: Tiny UX polish: wording.Suggest “Enter” instead of “enter key” and consistent punctuation.
- "\rWaiting for " - + seconds - + " seconds, press " - + (noBreak ? "CTRL+C to quit ..." : "enter key to continue ...")); + "\rWaiting for " + + seconds + + " seconds; press " + + (noBreak ? "CTRL+C to quit ..." : "Enter to continue ..."));
55-58: Clarify range message.You accept -1 (delegates to pause). Consider clarifying the error to “Valid values: -1 or 0–99999 seconds.”
20-22: Direct errors to stderr.Route error messages to
System.errfor better CLI ergonomics. Applies to all validation prints in this block.- System.out.println( + System.err.println(Also applies to: 30-32, 37-39, 44-45, 50-51, 55-57
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CheckList.md(1 hunks)src/main/java/com/mycmd/commands/TimeoutCommand.java(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: ⚙️ Java CI Build
src/main/java/com/mycmd/commands/TimeoutCommand.java
[error] 67-67: Compilation failed: cannot find symbol 'AtomicBoolean' in TimeoutCommand.java. Ensure to import java.util.concurrent.atomic.AtomicBoolean or add the needed dependency. Command: mvn -B -DskipTests=false package
🔇 Additional comments (1)
CheckList.md (1)
120-120: Checklist update verified—timeout feature is complete and properly implemented.The timeout item has been correctly marked as complete. Verification confirms:
- ✓ TimeoutCommand.java exists with all required methods (execute, description, usage)
- ✓ App.java registers TimeoutCommand at line 142
- ✓ Implementation supports configurable timeout duration via
/t <seconds>or positional argument- ✓ Optional
/nobreakflag disables Enter key interruption; default allows interruption- ✓ Supports
-1for infinite wait, delegating to PauseCommand
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/java/com/mycmd/commands/TimeoutCommand.java (2)
16-48: Reject unknown arguments; currently ignored.Unrecognized tokens pass through silently, producing surprising behavior. Fail fast with a clear error.
Apply this diff after the existing
else if (args[i].equalsIgnoreCase("/t"))branch:} else if (args[i].equalsIgnoreCase("/t")) { System.out.println("Error: Invalid syntax. Value expected for '/t'."); return; + } else { + System.out.println("Error: Invalid syntax. Unrecognized option: " + args[i]); + return; }
71-91: Stop and join the input thread; otherwise it can steal stdin after timeout completes.
System.in.read()is blocking and not reliably interruptible. If the countdown ends without Enter, the daemon input thread keeps reading and races the shell’s Scanner. Join it deterministically and accept CR/LF as Enter. Also drain stdin on the early-exit path.Suggested patches:
@@ - AtomicBoolean interrupted = new AtomicBoolean(false); - Thread inputThread = null; + AtomicBoolean interrupted = new AtomicBoolean(false); + AtomicBoolean stopInput = new AtomicBoolean(false); + Thread inputThread = null; @@ - if (!noBreak) { - inputThread = - new Thread( - () -> { - try { - int r; - // Read until a newline is encountered so we only exit on Enter - while ((r = System.in.read()) != -1) { - if (r == '\n') { - interrupted.set(true); - break; - } - } - } catch (IOException e) { - // Ignore: if System.in is closed or an I/O error occurs we - // cannot reliably wait for Enter; treat as no-interrupt. - } - }); - inputThread.setDaemon(true); - inputThread.start(); - } + if (!noBreak) { + inputThread = new Thread(() -> { + try { + while (!stopInput.get()) { + if (System.in.available() > 0) { + int r = System.in.read(); + // Treat CR or LF as Enter across platforms + if (r == '\n' || r == '\r') { + interrupted.set(true); + break; + } + } else { + try { + Thread.sleep(25); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + break; + } + } + } + } catch (IOException e) { + // Best-effort; fall through. + } + }); + inputThread.setDaemon(true); + inputThread.start(); + } @@ - if (!noBreak && interrupted.get()) { + if (!noBreak && interrupted.get()) { System.out.println("\r"); System.out.println(); - if (inputThread != null && inputThread.isAlive()) { - inputThread.interrupt(); - } + if (inputThread != null) { + stopInput.set(true); + try { + inputThread.join(200); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + } + } + // Drain any remaining bytes so the shell doesn't see leftovers + try { + while (System.in.available() > 0) { + System.in.read(); + } + } catch (IOException e) { + // Ignore; non-fatal. + } return; } @@ - try { + // Normal completion: stop the input thread before draining. + if (!noBreak && inputThread != null) { + stopInput.set(true); + try { + inputThread.join(200); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + } + } + try { while (System.in.available() > 0) { System.in.read(); } } catch (IOException e) { // Ignore: if we can't drain the stream it's non-fatal; any leftover input // will be handled by the next read and is acceptable. }Also applies to: 94-101, 122-132
🧹 Nitpick comments (1)
src/main/java/com/mycmd/commands/TimeoutCommand.java (1)
104-110: Optional: fully clear the line when the countdown shrinks.When going 10 -> 9 seconds, a trailing character can remain because only CR is used. Consider clearing the line before printing the next message (e.g., print “\r” + ANSI clear-line “\u001B[2K”, then the message). If ANSI isn’t desired, pad with spaces.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/mycmd/commands/TimeoutCommand.java(1 hunks)
🔇 Additional comments (2)
src/main/java/com/mycmd/commands/TimeoutCommand.java (2)
1-7: Good: import fixes are in place.Adding
AtomicBooleanimport resolves the prior CI failure. Nothing else needed here.
112-119: Interrupt semantics in /nobreak mode — confirm intent.Catching
InterruptedExceptionandcontinuein/nobreakswallows the interrupt status, potentially masking shutdown signals. Is this intentional? If not, consider preserving the flag or exiting the loop.
|
🚀 Hi @nihaltp! Thank you for contributing to MyCMD. A maintainer will review your PR shortly. 🎉 |
anshumanjadiya1102
added
hacktoberfest-accepted
2 participants
Closes #86
Summary by CodeRabbit
New Features
/nobreakoption to disable interruption./t <seconds>;-1delegates to existing pause behavior.Documentation