Repository navigation
Copy and run a chat code block from a hover strip - #985
Conversation
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. |
PR Summary by QodoAdd hover actions to copy and run chat code blocks
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo
1.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23d1d4e247
ℹ️ 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".
| var canRun = Observable.CombineLatest( | ||
| _input.WhenAnyValue(i => i.CanAcceptText), | ||
| this.WhenAnyValue(x => x.IsReadOnlyParticipant), | ||
| (can, readOnly) => can && !readOnly); |
There was a problem hiding this comment.
Block Run while another send is uploading
During an attachment upload, _input.CanAcceptText remains true because the input send has not started, even though SendCommand is already executing. This leaves Run enabled, and clicking it invokes SendCommand.Execute() directly, bypassing that command's CanExecute gate and starting another upload from the same tray. Whichever upload completes first can consume the attachments while the other send is rejected, so either the original prompt or the requested command can be lost; include the active send/upload state in this gate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 6ccb3c8. ScriptedInput.CanAcceptText is Pending is null, and SendAsync has not been called during the upload, so the gate really did stay open there. canRun now also takes SendCommand.IsExecuting.
Running_a_code_block_is_refused_while_a_send_is_uploading asserts the channel still reports CanAcceptText in that window while the run command refuses. I checked it fails without the new term.
| var block = scratch.Children[0]; | ||
| // A control carries one parent: the scratch panel has to let go before the host adopts it. | ||
| scratch.Children.Clear(); | ||
| renderer.WriteBlock(Host(block, obj.Lines.ToString().TrimEnd('\n', '\r'))); |
There was a problem hiding this comment.
Preserve intentional trailing blank lines in code blocks
For a fenced block that intentionally ends with one or more blank lines, TrimEnd('\n', '\r') removes every trailing line break rather than only the single terminator added by StringLineGroup.ToString(). Copy and Run therefore receive different text from the displayed block, which can change pasted multiline content; remove only the renderer-added final line ending.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6ccb3c8, though not for the stated reason — worth recording because it changes the fix.
StringLineGroup.ToString() adds no terminator at all. It delegates to ToSlice, which appends the newline between lines only (if (i > 0) { Append(newLine); }) and nothing after the last. So there was no renderer-added final line ending to remove selectively: the TrimEnd was pure over-trimming, and the fix is to drop it outright rather than narrow it.
The conclusion held regardless, so: obj.Lines.ToString() now goes through untouched. A_blank_last_line_survives_the_copy pins ```bash\nls -la\n\n``` copying as ls -la\n, and the existing multi-line test still copies cd /tmp\nls -la with no trailing newline — which is the evidence that nothing was being appended.
The strip wraps the block MarkView's own renderer wrote rather than replacing that renderer, so syntax highlighting and its theme switch stay with the package across an upgrade. A block reads the run command as it is built, so the view builds the document again when a binding delivers one after the first render. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
23d1d4e to
44e51e1
Compare
The run offer now matches the verbatim payload: the bang must be the text's own first character, and a blank last line the block shows survives the copy. Run carries the command itself, so an unavailable composer greys it rather than swallowing a click, and its gate takes in the send's executing state — through an upload the channel still reports it can take text. The strip hides by going transparent because an invisible one is skipped by tab navigation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Closes #984 — AI-2902
What & why
Text in a fenced code block cannot be lifted out of it, which bites hardest when the agent writes a
command it wants run: the only way into the composer was to retype it. Hovering a block now reveals
Copy, and Run it when the block's text begins with
!— the bang being what makes the line acommand rather than prose, so it is the whole condition. Run sends the text verbatim, bang included,
through the composer's own send path, so a block's prompt is queued, recalled and cleared exactly as
a typed one is; whatever draft was sitting there is replaced.
Where to look
The strip wraps the block MarkView's own renderer wrote instead of replacing that renderer, so
syntax highlighting and its theme switch stay with the package. A block reads
RunCodeas it isbuilt, which is why the view builds the document again when a binding delivers one after the first
render — the pull request reader, which never gets one, offers copy alone.
Verification
dotnet run --project test/Capacitor.App.Tests.Unit/...— 2270/2270 passed.dotnet build src/Capacitor.App/Capacitor.App.csproj --no-incremental— 0 warnings.The late-binding path was checked by removing the re-render and watching
A_run_command_set_after_the_text_still_reaches_the_blockfail, then restoring it.Not checked: the strip's colours and padding in a real window.
🤖 Generated with Claude Code