Ignore remaining commands after a foreground job stops #487
Conversation
This commit adds a check to `add_job_if_suspended` to break if the process is interactive. This implements most part of <#478>.
This change makes the `wait_until_halt`, `resume_job_by_index`, and `resume_job_by_id` functions in the `fg` built-in return `ProcessResult` instead of `ProcessState`. These function never return `ProcessState::Running`, so it is more appropriate to return `ProcessResult` instead.
WalkthroughThis pull request updates several modules to refine how the shell handles job control and error reporting. Function signatures in the foreground job management (fg) have been updated to return a richer Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Invoke as invoke_target
participant External as start_external_util
participant Control as ControlFlow Match
User->>Invoke: Execute external command
Invoke->>External: Start external utility in subshell
External-->>Invoke: Return result (Continue/Break)
alt Result is Continue
Invoke-->>User: Return exit status
else Result is Break
Invoke-->>User: Return status with divert
end
sequenceDiagram
participant User
participant fgMain as Shell fg::main
participant Process as Job Process
participant JobUtil as add_job_if_suspended
User->>fgMain: Execute foreground command
fgMain->>Process: Wait/resume job execution
Process-->>fgMain: Return ProcessResult (Running/Stopped/Exited)
alt ProcessResult indicates suspension
fgMain->>JobUtil: Handle suspended job
JobUtil-->>fgMain: Return Break(divert) or Continue(status)
else ProcessResult normal
fgMain-->>User: Return exit status
end
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (11)
🧰 Additional context used🧬 Code Definitions (4)yash-builtin/src/command/invoke.rs (2)
yash-semantics/src/command/pipeline.rs (1)
yash-semantics/src/command/simple_command/absent.rs (1)
yash-builtin/src/fg.rs (2)
🔇 Additional comments (36)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
When a job suspends in the `fg` built-in, the built-in now returns with `Divert::Interrupt` if the current environment is interactive. This completes the implementation of <#478>.
magicant
force-pushed
the
job-suspension
branch
from
5b69aec to
08f623c
Compare
Closes #478
This pull request includes several changes to improve the handling of job control and process states in the
yashshell. The most significant changes involve updating various functions to use theProcessResulttype instead ofProcessState, and introducing a utility function to handle suspended jobs. Additionally, there are updates to the changelogs and the use of theadd_job_if_suspendedfunction across multiple files.Improvements to job control and process state handling:
yash-builtin/src/command/invoke.rs: Updated theinvoke_targetfunction to handleBreakandContinuecontrol flow for external utilities.yash-builtin/src/fg.rs:wait_until_halt,resume_job_by_index, andresume_job_by_idfunctions to useProcessResultinstead ofProcessState. [1] [2] [3] [4]ProcessResultinstead ofProcessState. [1] [2] [3]Introduction of utility function for suspended jobs:
yash-semantics/src/command/compound_command/subshell.rs: Replaced manual job handling with theadd_job_if_suspendedutility function.yash-semantics/src/command/pipeline.rs: Updated theexecute_job_controlled_pipelinefunction to useadd_job_if_suspended.yash-semantics/src/command/simple_command/absent.rsandexternal.rs: Replaced manual job handling withadd_job_if_suspended. [1] [2]Changelog updates:
yash-cli/CHANGELOG.md: Noted changes in behavior when a foreground job is suspended in an interactive shell.yash-semantics/CHANGELOG.md: Added details about new functions and changes in job handling.These changes enhance the robustness of job control in the
yashshell and simplify the code by centralizing job suspension handling.Summary by CodeRabbit
New Features
Bug Fixes
Tests