Open pull requests from Pane, and follow them from the review panel - #387
Open
0x92 wants to merge 4 commits into
Open
Open pull requests from Pane, and follow them from the review panel#3870x92 wants to merge 4 commits into
0x92 wants to merge 4 commits into
Conversation
Reviewing a session meant reading one combined patch: which files a commit touched, and how much, was not visible anywhere. Two changes, both in the diff path: - Commit history rows expand to a per-file list with status and line counts, and clicking a file opens that file's diff directly - Reviewing a large working tree no longer blocks the UI The freeze was real work, not a hang: capturing the working-directory diff spawned one `cat` per untracked file and one `wc -l` per file, all synchronously on the main thread — 800+ spawns for a session with many untracked files, several seconds each pass. The path is now async and batched, with the file list gathered once instead of per consumer. The parsers for `--numstat -z`, `--name-status -z` and porcelain `-z` are pure functions with their own tests; the `-z` format packs `XY path` into a single token, which is easy to get wrong and impossible to notice by eye. Tests: commit file changes (23) and the unified diff parser (11).
Two things went wrong for WSL projects, both because the host is Windows while the shell that runs the command is bash inside the distro. Arguments were quoted with escapeShellArg(), which picks its style from process.platform and therefore handed bash double quotes. Inside those, $, backticks and \ stay live. Git only forbids space, ~^:?*[\ and control characters in a ref, so a branch named fix-`whoami` is legal and would have run as a command; the pull request title is free user text on the same path. quoteArg() now asks the CommandRunner which shell it targets and uses escapeForBash() from wslUtils for WSL, which exists for exactly this and says so in its comment. The pull request body was written to the host tmpdir() and its C:\ path passed to a gh running inside the distro, which cannot open it, so --body-file failed before a pull request was ever created. resolveBodyFile() now writes to the distro's /tmp through the UNC view Windows has of it and names the Linux path to gh. Host projects keep the behaviour they had.
Two defects, both in the path that inlines untracked files into a working-tree diff. The listing used `git ls-files --others --exclude-standard` split on newlines. Git delimits with newlines there, which a filename may contain, and quotes anything non-ASCII into a C-style escape — `täst.txt` arrived as the literal `"t\303\244st.txt"`, a name matching no file on disk, so the file vanished from the diff and from the stats without a word. The listing now uses `-z` and is split on NUL, and nothing is trimmed: a leading or trailing space is part of the name. Content and line counts were then read with `cat "<worktree>/<file>"` and `wc -l "<worktree>/<file>"`, built by interpolation. Git allows `$`, backticks and parentheses in a filename, and inside bash double quotes those are still syntax: a file named `back`whoami`.txt` in a repository was enough to run a command, with no interaction beyond opening the session. Both now go through `fs` — readFile for content, a streamed newline count for the totals — so a repository-controlled name never reaches a shell. untrackedFilePath() resolves the worktree-relative name git reports, going through the UNC mount for a WSL project the same way gitPlumbingCommands already does. The performance fix this PR made is kept and improved: capture spawns a fixed handful of commands whatever the file count, where before it was one `cat` and one batched `wc` per file. MAX_UNTRACKED_INLINE_FILES and MAX_UNTRACKED_INLINE_BYTES still bound what is inlined, and a per-file ceiling stands in for the 1 MB buffer `cat` used to have. chunkByCommandLength() goes with them — nothing builds a command line out of paths any more.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Open a pull request for a session's branch without leaving Pane, and follow it from the review panel afterwards.
A session already is a branch in a worktree, so everything a pull request needs is on hand. Today the flow leaves the app: switch to a browser, find the fork, remember which repository the contribution is meant for. This adds a Create Pull Request entry to the session's git menu that gathers the branch, its commits, the repository's PR template and the possible targets in one round trip, pushes the branch and calls
gh pr create.Once the pull request exists, the same session's Review panel gains a status strip: state, review decision, mergeability, size and reviewers — the questions that otherwise send you back to the browser.
What it does
Creating
pull_request_template.mdis appended rather than replacing them.owner:branchwhen the pull request crosses repositories.<datalist>hides everything that does not match what is already typed.)+/-and totals, compared against the merge base the way GitHub counts it. "Show diff" loads the patch on demand and renders it in the existingDiffViewer.ghmissing or signed out, an existing pull request for this branch (shown as a link — there is nothing to create), a base branch the target repository does not have.Following
Open / Draft / Merged / Closed, number and title, review decision, comment count, and a conflict warning when GitHub reportsCONFLICTING.owner:head → repo:base, size, and each reviewer with their newest verdict — a "changes requested" that the same person later approved is not an open objection.How it works
PullRequestManager(main process) is the only place that runs commands; everything that decides what to send is a pure function next to it, tested on its own:deriveDraftText,resolveTargets,buildCreateArgs,normalizeBaseBranch,sortBaseBranches,parsePullRequestStatus,parseChecks, …ghis invoked through the session'sCommandRunner, so a WSL project uses theghinside the distro rather than the one on the Windows host. Whenghis not on the snapshotted PATH — for instance because it was installed while Pane was running — the usual install locations are tried before giving up.--body-file, so backticks, quotes and newlines in markdown survive.pr:*): the branch, its remote and theghbinary live on the machine that runs the agents, so a remote runtime answers these itself instead of having the laptop push a branch it does not have.Type of Change
Checklist
pnpm typecheckandpnpm lintlocallypnpm electron-devCritical Areas Modified
Additional Notes
Builds on the per-file commit details PR — it reuses
shared/types/git.ts, thenumstat/name-statusparsers ingitDiffManagerandparseUnifiedDiff. Please merge that one first.Tested against the real GitHub CLI and this repository: draft generation, the target list for a fork, base-branch listing across several API pages, the file comparison cross-checked against
git diff --name-only <base>...HEAD, and status for open pull requests. One real pull request was created end to end and closed again.