docs(delegate): explain --no-pager in PTY environments - #1062
Conversation
|
✅ OpenCodeReview: Review skipped: no items were selected. |
Qiyuanqiii
left a comment
There was a problem hiding this comment.
Self-review — technical writing pass
The Git commands and pager behavior are correct, but I found two wording issues to address before marking this PR ready:
-
In both
SKILL.mdcopies, “so a PTY-based agent environment does not start an interactive pager and wait for input” assigns both actions to the environment and usesPTYwithout defining it. Git launches the pager; the pager waits. Suggested wording:Always pass
--no-pagerwhen reading diffs or file content through Git. Otherwise, in an agent environment that allocates a pseudo-terminal (PTY), Git may launch an interactive pager that waits for input. -
“Inspect the stat” is ambiguous. Git calls the
--statoutput a diffstat. Suggested wording:--no-pagerdoes not bound the amount of output captured by the host. For a potentially large diff, inspect the--statoutput first, redirect the full diff to a temporary file, and read it in bounded chunks.
The PR description should likewise replace “does not limit stdout size” with “does not limit the amount of output written to stdout.” It should also attribute status 137 to Unix shells or CI runners (128 + SIGKILL) rather than to Unix hosts generally.
No functional or command-level issues were found. The canonical and plugin skill copies remain synchronized except for the plugin mirror notice.
References: Git global options, Git diffstat terminology, and Bash exit-status semantics.
|
Addressed the technical-writing findings in
Both skill copies remain synchronized, and |
|
Just like #1061, I’ve also completed my review. You can take a look now. If there are problems, I’ll handle them after I get up around noon. |
Description
Summary
git --no-pagerin every delegation example that reads a diff or commit through Git.Root Cause and Fix
Step 3 of the delegation skill previously instructed host agents to run
git diffandgit showdirectly. When an agent or CI runner allocates a pseudo-terminal (PTY), Git can treat stdout as interactive and launch its configured pager, usuallyless. The pager may then wait for input until the host terminates the process.The examples now explicitly disable pagination:
git diff <merge_base>..<to> -- <path>git --no-pager diff <merge_base>..<to> -- <path>git show <commit> -- <path>git --no-pager show <commit> -- <path>git diff HEAD -- <path>git --no-pager diff HEAD -- <path>Following review feedback, the skill contains only the actionable one-line reason for the flag. The detailed PTY, timeout, and exit-code explanation remains in this PR description rather than being presented to the agent during every review.
Disabling pagination solves the interactive wait but does not limit the amount of output written to stdout. The skill therefore also recommends inspecting the
--statoutput first, redirecting a potentially large diff to a temporary file, and reading it in bounded chunks.Compatibility
--no-pageris a standard Git global option and is placed before thedifforshowsubcommand.Type of Change
How Has This Been Tested?
Reproduction Environment
2.54.0.windows.1less 692cfbb62e2296d7684dc648e27fe1b906e8c960f9cRed/Green Reproduction
With
TERM=xterm,GIT_PAGER=less, a PTY, and a four-second timeout:git diff ...less, remained blocked, and was terminated after 4.083 secondsgit --no-pager diff ...The PowerShell/MSYS process boundary represented the forced termination as exit 2304. Unix shells and CI runners commonly report
SIGKILLas status 137 (128 + 9), as reported in #1059. The important red/green result is the pager wait versus immediate completion.Final Validation
mingw32-make checkpassed after the latest documentation change: license check, English-only check,go mod tidy,gofmt -s -w ., andgo vetmake testequivalent passed withGOFLAGS=-p=1to constrain local package build concurrencygit diff --checkpassedgit add --renormalize .ff1e2d6Checklist
Known Limitations
--no-pagerfor Git commands an agent constructs outside these examples.--no-pagerprevents an interactive pager wait but does not limit the amount of output written to stdout; bounded temporary-file reads remain necessary for large diffs.Related Issues
Closes #1059.