Repository navigation
feat(core): 回复光标位置报告 DSR 6n (#28) - #224
Conversation
Parser keeps a bounded reply queue for CSI 6n, the shell forwards replies through the same input queue as keyboard bytes, and reply_test covers the 1-based report plus the unimplemented-report negative.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4923a1782
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #ifndef TERMFORGE_CANDIDATE_CORE | ||
| // Query replies (for example a cursor position report) go back to the | ||
| // same bounded input queue as keyboard bytes, so ordering and | ||
| // backpressure stay identical for the child. | ||
| for (const auto& reply : parser_.take_replies()) input_.send(reply); |
There was a problem hiding this comment.
In the confirmed C++/Qt/fixed-ConPTY product configuration, src/qt/terminal_session.hpp::poll() feeds this same Parser but never calls take_replies(); the only reply consumer added here is in the legacy Win32 shell. Consequently, a child issuing CSI 6n in the Qt application still receives no response, and repeated queries now accumulate reply strings indefinitely in the session's parser. Drain and send replies from the Qt session's output-consumption path as well, with an explicit policy for input rejection. This also follows AGENTS.md's requirement to implement the confirmed Qt product route.
AGENTS.md reference: AGENTS.md:L5-L6
Useful? React with 👍 / 👎.
| // Query replies (for example a cursor position report) go back to the | ||
| // same bounded input queue as keyboard bytes, so ordering and | ||
| // backpressure stay identical for the child. | ||
| for (const auto& reply : parser_.take_replies()) input_.send(reply); |
There was a problem hiding this comment.
When a query arrives during an active paste or while the input queue is full, InputQueue::send() returns false (experiments/shell/queues.hpp explicitly rejects both cases). This loop ignores that result after take_replies() has removed the replies from the parser, so the cursor report is permanently lost and a child awaiting it can stall or time out. The queue therefore provides rejection rather than the claimed backpressure here. Retain rejected replies for later delivery or explicitly surface the failure instead of silently continuing.
Useful? React with 👍 / 👎.
The recovery probe counts handled reports separately now, so the unknown-CSI case expects two ignored sequences and a new case pins the cursor report as handled.
…refs #28 The core answers CSI 6n now, so the matrix compares those records instead of marking them unsupported and the expected counts move five cases to match.
内容
为 #28 补上产品核心的 DSR（
CSI 6n）回复，语义对齐参考实现CandidateTerminal：Parser增加有界回复队列：CSI 6n→ESC[<row>;<col>R（1 基）；其它 DSR 编号不走这条路径，仍按"不支持"处理，不伪造；take_replies()，由宿主在每次feed()后取走；tick()消费输出后把回复写入输入队列（与键盘同一队列，保持顺序与背压）。新增测试
experiments/core/reply_test.cpp（CTestreply_core）：feed("ab")后CSI 6n→ 唯一回复ESC[1;3R；CSI 2;7H后查询 →ESC[2;7R；CSI 5n（未实现的报告）→ 无回复。本地运行：
reply_test输出cursor reply contracts passed，exit 0。边界
TERMFORGE_CANDIDATE_CORE）走既有事件路径不受影响。Refs #28