Feat improve terminal - #61
Conversation
|
Warning Review limit reached
Next review available in: 24 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough本次变更将 TTY 输入改为 termios 行规程，增加终端信号、窗口大小和动态设备元数据处理，并更新标准流路径、ioctl、轮询和 ChangesTTY 行规程与读取流程
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant InputSource
participant TtyInputState
participant ForegroundProcessGroup
participant StdinObject
InputSource->>TtyInputState: 处理输入字符
TtyInputState->>ForegroundProcessGroup: 发送终端信号
TtyInputState->>StdinObject: 写入可读数据或 EOF
StdinObject->>TtyInputState: 按 termios、VMIN 和 VTIME 读取
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
pulse_core/src/fd_table/tty.rs (4)
259-273: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win建议在信号字符触发时清空输入缓冲。
当前实现在命中 VINTR/VQUIT/VSUSP 后直接返回，
input.canonical中已缓冲的字符保持不变。POSIX 行为是：如果未设置NOFLSH，终端在发送信号时刷新输入队列。因此按下 Ctrl+C 后，之前输入的半行内容会在下一次提交时一并交付给应用，与真实终端不一致。♻️ 建议的修改
if let Some(signal) = signal { + if (termios_data.c_lflag & NOFLSH) == 0 { + input.canonical.clear(); + input.readable.clear(); + } effects.signals.push(signal); return effects; }需要在文件顶部的
linux_raw_sys::general导入中加入NOFLSH。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pulse_core/src/fd_table/tty.rs` around lines 259 - 273, 在信号字符处理逻辑中检查 termios_data.c_lflag 的 NOFLSH 标志：当 VINTR、VQUIT 或 VSUSP 触发信号且未设置 NOFLSH 时，清空 input.canonical 后再返回 effects；设置 NOFLSH 时保留缓冲内容。同步在 linux_raw_sys::general 导入中加入 NOFLSH。
670-676: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win当
/dev/tty查找失败时，fstat(0)/fstat(1)会失败。旧实现为 stdin/stdout 返回固定的 stat 值，总是成功。现在如果 devfs 尚未挂载或
/dev/tty不存在，console_tty_stat会把错误传播给fstat、sys_statx与pulse_core/src/task/mod.rs中的fd_path（后者会回退为/dev/null）。建议在查找失败时回退到一个字符设备形态的合成 stat，保持标准流的fstat始终可用。另外，每次
stat()都执行一次lookup_location路径解析。如果调用频繁，可以缓存该Location。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pulse_core/src/fd_table/tty.rs` around lines 670 - 676, Update console_tty_stat to fall back to a synthetic character-device stat when axfs::lookup_location("/dev/tty") fails, preserving successful fstat behavior for standard streams even before devfs is mounted; keep the existing location_to_stat path when lookup succeeds. Do not propagate the lookup error to fstat/sys_statx or fd_path.
946-977: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议补充输入行规程的测试用例。
现有测试覆盖了规范模式的换行提交、擦除，以及非规范模式的即时可读和信号字符。以下分支仍无覆盖，且都属于易出错的逻辑：
VEOF：空行时置eof_pending，非空行时提交当前行。VKILL：清空整行并按ECHOK回显换行。map_input_byte：ICRNL、INLCR、IGNCR三种映射。TtyInputState::read_into：EOF 只消费一次的语义。
read_noncanonical_input的 VMIN/VTIME 组合依赖全局状态与等待队列，难以直接单测。如果把它重构为接受输入状态与等待原语的参数，就可以覆盖。需要我生成这些测试吗？🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pulse_core/src/fd_table/tty.rs` around lines 946 - 977, 为输入行规程补充测试，覆盖 VEOF 在空行设置 eof_pending、非空行提交当前行，VKILL 清空整行并在 ECHOK 启用时回显换行，以及 map_input_byte 对 ICRNL、INLCR、IGNCR 的映射；同时验证 TtyInputState::read_into 的 EOF 只被消费一次。围绕 accept_input_byte、map_input_byte 和 read_into 添加用例，暂不重构依赖全局状态的 read_noncanonical_input。
426-433: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
TTY_TERMIOS的读-改-写未原子化。第 426 行获取快照后立即释放锁，
update_tty_termios在第 433 行重新加锁。两个并发的TCSETS可能互相覆盖字段。可以让update_tty_termios接受一个闭包，在单次持锁期间完成修改。影响有限，因为并发设置 termios 很少见。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pulse_core/src/fd_table/tty.rs` around lines 426 - 433, 将 TTY_TERMIOS 的读改写流程改为原子操作：修改 update_tty_termios，使其接受闭包并在同一次持锁期间读取当前 termios、执行字段更新并写回；同步调整当前调用处，避免先通过 TTY_TERMIOS.lock() 获取快照后再单独调用 update_tty_termios，从而防止并发 TCSETS 覆盖彼此的修改。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pulse_core/src/fd_table/tty.rs`:
- Around line 259-273: 在信号字符处理逻辑中检查 termios_data.c_lflag 的 NOFLSH 标志：当
VINTR、VQUIT 或 VSUSP 触发信号且未设置 NOFLSH 时，清空 input.canonical 后再返回 effects；设置 NOFLSH
时保留缓冲内容。同步在 linux_raw_sys::general 导入中加入 NOFLSH。
- Around line 670-676: Update console_tty_stat to fall back to a synthetic
character-device stat when axfs::lookup_location("/dev/tty") fails, preserving
successful fstat behavior for standard streams even before devfs is mounted;
keep the existing location_to_stat path when lookup succeeds. Do not propagate
the lookup error to fstat/sys_statx or fd_path.
- Around line 946-977: 为输入行规程补充测试，覆盖 VEOF 在空行设置 eof_pending、非空行提交当前行，VKILL
清空整行并在 ECHOK 启用时回显换行，以及 map_input_byte 对 ICRNL、INLCR、IGNCR 的映射；同时验证
TtyInputState::read_into 的 EOF 只被消费一次。围绕 accept_input_byte、map_input_byte 和
read_into 添加用例，暂不重构依赖全局状态的 read_noncanonical_input。
- Around line 426-433: 将 TTY_TERMIOS 的读改写流程改为原子操作：修改
update_tty_termios，使其接受闭包并在同一次持锁期间读取当前 termios、执行字段更新并写回；同步调整当前调用处，避免先通过
TTY_TERMIOS.lock() 获取快照后再单独调用 update_tty_termios，从而防止并发 TCSETS 覆盖彼此的修改。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a16cf15-a837-4eb2-aece-dbff53b72c5b
📒 Files selected for processing (5)
pulse_core/src/fd_table/mod.rspulse_core/src/fd_table/tty.rspulse_core/src/task/mod.rspulse_syscalls/src/impls/fs/control.rspulse_syscalls/src/impls/fs/meta.rs
* fix(tty): implement console line discipline * fix(tty): expose terminal identity and window ioctls * fix(tty): honor signal flush and stdio stat fallback
1 participant
Summary by CodeRabbit
新功能
问题修复
/dev/tty。