Compare commits

...

2 Commits

Author SHA1 Message Date
nntrivi2001 4b8f59695b chore: v2.8.44 — Discard in the Changes panel takes only the ticked files and blocks 2026-10-30 13:05:48 +07:00
nntrivi2001 da7b86d5c8 fix(changes): Discard takes only the ticked files and blocks (#87)
The Changes panel's "Discard all" threw away every changed file, ticked or
not. It is now "Discard (n)": the same selection Commit sends — whole ticked
files as `paths`, a partly ticked file's ticked blocks as `hunks` — and is
disabled with nothing ticked. The server's `discard` accepts `hunks`
(`selectionPatch` in reverse, `git apply -R`), building every patch before
applying any so a stale selection undoes nothing.

Closes #87
2026-10-30 13:05:13 +07:00
12 changed files with 182 additions and 54 deletions
+2 -2
View File
File diff suppressed because one or more lines are too long
@@ -1470,25 +1470,40 @@ export function LaneChangesPanel({
}
};
// Discard all: every non-conflicted file in one `discard`, asked once in a
// dialog that names how many untracked files it will delete.
const discardAll = async () => {
const rows = files.filter((f) => !f.conflicted);
if (!rows.length) return;
// Discard selected: exactly what Commit would take — the ticked files, and
// only the ticked blocks of a partly ticked one (#87: it used to discard
// every file, ticked or not) — in one `discard`, asked once in a dialog
// that names how many untracked files it will delete.
const discardTicked = async () => {
if (!tickedPaths.length) return;
const ok = await confirmDialog({
title: t("changes.discardAllTitle"),
message: t("changes.discardAllConfirm", {
count: rows.length,
untracked: rows.filter((f) => f.untracked).length,
title: t("changes.discardTickedTitle"),
message: t("changes.discardTickedConfirm", {
count: tickedPaths.length,
untracked: files.filter((f) => f.untracked && tickedPaths.includes(f.path)).length,
}),
confirmLabel: t("changes.discardAll"),
confirmLabel: t("changes.discardTicked"),
});
if (ok)
await act(
"discard",
rows.map((f) => f.path),
true
);
if (!ok) return;
const lane = laneId;
setBusy(true);
setError(null);
try {
const { paths, hunks } = await selection();
await api.lanes.gitAction(target, "discard", {
paths,
...(hunks ? { hunks, context: CONTEXT_STEPS[0] } : {}),
confirm: true,
});
if (laneNow.current !== lane) return;
setConfirmDiscard(null);
refreshGit();
await load();
} catch (err) {
if (laneNow.current === lane) setError(err instanceof Error ? err.message : String(err));
} finally {
if (laneNow.current === lane) setBusy(false);
}
};
// resolve / merge-commit / merge-abort: same shape as `act`, other body.
@@ -2067,8 +2082,7 @@ export function LaneChangesPanel({
// An HTML page's relative CSS/scripts/images, read like a download — only
// from the working tree: a commit's page would get today's files.
const htmlFetch = useMemo(
() =>
range === "commit" ? null : fetchVia((p) => api.lanes.gitDownloadUrl(target, p)),
() => (range === "commit" ? null : fetchVia((p) => api.lanes.gitDownloadUrl(target, p))),
// eslint-disable-next-line react-hooks/exhaustive-deps
[range, JSON.stringify(target)]
);
@@ -2863,13 +2877,13 @@ export function LaneChangesPanel({
<button
type="button"
data-testid="lane-changes-discard-all"
disabled={busy}
onClick={() => void discardAll()}
title={t("changes.discardAllTitle")}
disabled={busy || tickedPaths.length === 0}
onClick={() => void discardTicked()}
title={t("changes.discardTickedTitle")}
className="shrink-0 flex items-center gap-1 rounded border border-border-light px-1.5 py-0.5 text-[11px] text-status-danger hover:bg-status-danger/10 disabled:cursor-not-allowed disabled:opacity-40"
>
<Trash2 className="h-3 w-3" aria-hidden />
{t("changes.discardAll")}
{t("changes.discardTicked", { count: tickedPaths.length })}
</button>
</div>
</div>
@@ -837,10 +837,12 @@ describe("LaneChangesPanel", () => {
});
});
it("discards every file at once only after the dialog is confirmed", async () => {
it("discards only the ticked files, and only after the dialog is confirmed (#87)", async () => {
const user = userEvent.setup();
render(<LaneChangesPanel laneId={7} />);
await screen.findByTestId("lane-changes-overview");
// notes.txt (unversioned) starts unticked; untick docs/API.md too.
await user.click(tickOf("docs/API.md"));
vi.mocked(confirmDialog).mockResolvedValueOnce(false);
await user.click(screen.getByTestId("lane-changes-discard-all"));
@@ -849,10 +851,14 @@ describe("LaneChangesPanel", () => {
await user.click(screen.getByTestId("lane-changes-discard-all"));
await waitFor(() =>
expect(api.lanes.gitAction).toHaveBeenCalledWith(7, "discard", {
paths: ["server/lib/git-ops.js", "notes.txt", "docs/API.md"],
paths: ["server/lib/git-ops.js"],
confirm: true,
})
);
// Nothing ticked: nothing to discard.
await user.click(tickOf("server/lib/git-ops.js"));
expect(screen.getByTestId("lane-changes-discard-all")).toBeDisabled();
});
it("right-click on a changed file offers Commit File, a two-click discard, copy path and patch", async () => {
@@ -1673,6 +1679,25 @@ describe("LaneChangesPanel", () => {
expect(api.lanes.gitAction).not.toHaveBeenCalled();
});
it("discards only the ticked change blocks of a partly ticked file (#87)", async () => {
const user = userEvent.setup();
await openReview(user);
await user.click(
screen.getAllByRole("checkbox", { name: "Include this change in the commit" })[1]!
);
await user.click(screen.getByRole("button", { name: "Changes" }));
await screen.findByTestId("lane-changes-commit");
await user.click(screen.getByTestId("lane-changes-discard-all"));
await waitFor(() =>
expect(api.lanes.gitAction).toHaveBeenCalledWith(7, "discard", {
paths: [],
hunks: { "server/lib/git-ops.js": [{ hunk: "@@ -1,2 +1,3 @@", blocks: [0] }] },
context: 3,
confirm: true,
})
);
});
it("commits only the ticked change blocks of a partly ticked file (hunks)", async () => {
const user = userEvent.setup();
await openReview(user);
+3 -3
View File
@@ -532,9 +532,9 @@
"changes.unversionedGroup": "Unversioned Files",
"changes.commitTicked": "Commit ({{count}})",
"changes.commitAndPush": "Commit and Push",
"changes.discardAll": "Discard all",
"changes.discardAllTitle": "Discard all changes?",
"changes.discardAllConfirm": "Every change in {{count}} file(s) goes back to the last commit, and {{untracked}} new file(s) git does not track yet are deleted. This cannot be undone.",
"changes.discardTicked": "Discard ({{count}})",
"changes.discardTickedTitle": "Discard the ticked changes?",
"changes.discardTickedConfirm": "The ticked changes in {{count}} file(s) go back to the last commit, and {{untracked}} ticked new file(s) git does not track yet are deleted. Unticked files and blocks are kept. This cannot be undone.",
"changes.amendAndPush": "Amend and Push",
"changes.pushFailed": "Committed, but the push failed: {{error}}",
"changes.tickFile": "Include {{path}} in the commit",
+3 -3
View File
@@ -532,9 +532,9 @@
"changes.unversionedGroup": "File chưa theo dõi",
"changes.commitTicked": "Commit ({{count}})",
"changes.commitAndPush": "Commit và Push",
"changes.discardAll": "Bỏ tất cả",
"changes.discardAllTitle": "Bỏ tất cả thay đổi?",
"changes.discardAllConfirm": "Mọi thay đổi trong {{count}} file sẽ trở về commit cuối, và {{untracked}} file mới chưa được git theo dõi sẽ bị xoá. Không thể hoàn tác.",
"changes.discardTicked": "Bỏ ({{count}})",
"changes.discardTickedTitle": "Bỏ các thay đổi đã chọn?",
"changes.discardTickedConfirm": "Các thay đổi đã chọn trong {{count}} file sẽ trở về commit cuối, và {{untracked}} file mới đã chọn chưa được git theo dõi sẽ bị xoá. File và đoạn không chọn được giữ nguyên. Không thể hoàn tác.",
"changes.amendAndPush": "Amend và Push",
"changes.pushFailed": "Đã commit, nhưng push thất bại: {{error}}",
"changes.tickFile": "Đưa {{path}} vào commit",
@@ -7316,7 +7316,7 @@ exports[`screen snapshots > Settings — about 1`] = `
<span
class="text-status-warning/90"
>
UI build v2.8.43 (rebuild client to match)
UI build v2.8.44 (rebuild client to match)
</span>
</p>
</div>
+1 -1
View File
@@ -1063,7 +1063,7 @@ cannot collide on `index.lock`.
| `push` | `git push -u <remote> HEAD` — the branch's upstream remote, else the only remote, else `origin`, else `400 ENOREMOTE` | — |
| `stage` | `git add -- <paths>` | `{"paths": ["..."]}` |
| `unstage` | `git restore --staged -- <paths>` (a staged rename also passes its original path, so both halves unstage) | `{"paths": ["..."]}` |
| `discard` | `git restore --source=HEAD --staged --worktree` (tracked) / `git clean -fd` (untracked) | `{"paths": ["..."], "confirm": true}` |
| `discard` | `git restore --source=HEAD --staged --worktree` (tracked) / `git clean -fd` (untracked); with `hunks` (a partly ticked file's ticked change blocks, named as in `commit`), `git apply -R` of only those blocks off the working tree first — every patch built before any is applied, so a stale one is `409 EHUNKGONE` with nothing undone | `{"paths": ["..."], "confirm": true}` / `{"paths": [], "hunks": {"<path>": [{"hunk": "@@ … @@", "blocks": [0]}]}, "context": 3, "confirm": true}` |
| `stage-hunk` | `git apply --cached` of ONE hunk of `git diff -U<context> HEAD -- <path>` — or, with `block`, of only that hunk's `block`-th run of +/- lines (the other runs folded into context, as an edited `git add -p` hunk) | `{"path": "...", "hunk": "@@ -1,2 +1,3 @@", "block": 0, "context": 3}` |
| `discard-hunk` | `git apply -R` of that one hunk (or that one `block` of it), off the working tree | `{"path": "...", "hunk": "@@ ... @@", "block": 0, "context": 3, "confirm": true}` |
| `commit` | `git add -A` then `git commit -m <message>`, or the index as it stands with `only: "staged"`, or — with `paths` — `git commit --only -- <paths>`: exactly those files' working-tree content (an untracked one is added first, and taken back out if the commit fails), every other change left as it was | `{"message": "...", "only": "staged"}` / `{"message": "...", "paths": ["..."]}` |
+2 -2
View File
@@ -1,12 +1,12 @@
{
"name": "agenthub",
"version": "2.8.43",
"version": "2.8.44",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "agenthub",
"version": "2.8.43",
"version": "2.8.44",
"hasInstallScript": true,
"license": "UNLICENSED",
"dependencies": {
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "agenthub",
"version": "2.8.43",
"version": "2.8.44",
"bin": {
"agenthub": "bin/agenthub.js"
},
+45
View File
@@ -851,6 +851,51 @@ describe("runGitAction — one change block inside a hunk (issue #50)", () => {
assert.equal(fs.readFileSync(path.join(LANE, "tail.txt"), "utf8"), "l1\nl2\nl3\nl4\nl5\nl6");
});
it("discards only the ticked blocks and whole files, nothing unticked (issue #87)", async () => {
const edited = threeBlocks();
write(LANE, "keep.txt", "untouched by discard\n");
write(LANE, "drop.txt", "gone\n");
const [hunk] = await hunkOf();
await assert.rejects(
() =>
runGitAction(LANE, "discard", {
paths: ["drop.txt"],
hunks: { "near.txt": [{ hunk: hunk.header, blocks: [0, 2] }] },
}),
{ code: "ECONFIRM" }
);
await runGitAction(LANE, "discard", {
paths: ["drop.txt"],
hunks: { "near.txt": [{ hunk: hunk.header, blocks: [0, 2] }] },
confirm: true,
});
const want = [...edited];
want.splice(11, 0, "line 11");
want.splice(3, 2, "line 4", "line 5");
assert.deepEqual(read(), [...want, ""]);
assert.equal(fs.existsSync(path.join(LANE, "drop.txt")), false);
assert.equal(fs.readFileSync(path.join(LANE, "keep.txt"), "utf8"), "untouched by discard\n");
});
it("discards ticked blocks alone, and refuses a stale one with nothing undone", async () => {
const edited = threeBlocks();
const [hunk] = await hunkOf();
await assert.rejects(
() =>
runGitAction(LANE, "discard", {
hunks: { "near.txt": [{ hunk: "@@ -1,2 +1,2 @@", blocks: [0] }] },
confirm: true,
}),
{ code: "EHUNKGONE" }
);
assert.deepEqual(read(), [...edited, ""]);
await runGitAction(LANE, "discard", {
hunks: { "near.txt": [{ hunk: hunk.header, blocks: [1] }] },
confirm: true,
});
assert.deepEqual(read(), [...edited.filter((l) => l !== "INSERTED"), ""]);
});
it("refuses a block the hunk does not have", async () => {
threeBlocks();
const [hunk] = await hunkOf();
+5 -3
View File
@@ -1241,16 +1241,18 @@ async function diffHunks(dir, file, context) {
* in file order, to be applied FORWARD onto an index holding HEAD. Every
* header is matched against a fresh diff like `hunkPatch`'s; null when one is
* gone or names a block the hunk lacks. Each hunk's new-side start is left as
* git wrote it — `git apply` locates a hunk by its old side.
* git wrote it — `git apply` locates a hunk by its old side. `reverse` cuts
* it for `apply -R` off the working tree instead (the partial discard, #87),
* where the NEW side is the one checked and every start stays git's own.
*/
async function selectionPatch(dir, file, context, picks) {
async function selectionPatch(dir, file, context, picks, reverse = false) {
const diff = await diffHunks(dir, file, context);
if (!diff || !picks.length) return null;
const at = (header) => diff.hunks.findIndex((h) => h.slice(0, h.indexOf("\n")) === header);
if (picks.some((p) => at(p.hunk) < 0)) return null;
const bodies = [];
for (const p of [...picks].sort((x, y) => at(x.hunk) - at(y.hunk))) {
const body = onlyBlock(diff.hunks[at(p.hunk)], p.blocks, false);
const body = onlyBlock(diff.hunks[at(p.hunk)], p.blocks, reverse);
if (!body) return null;
bodies.push(body);
}
+56 -14
View File
@@ -424,6 +424,13 @@ async function runGitAction(dir, action, body = {}) {
if (action === "discard") {
if (body.confirm !== true) throw bad("ECONFIRM", "discard requires confirm: true");
// Ticked change blocks of partly ticked files (#87) go first: each patch
// is built before any is applied, so a stale one refuses before the
// irreversible whole-file part below has run.
const partial = hasHunks(body) ? await discardSelection(dir, body) : null;
if (partial && !(Array.isArray(body.paths) && body.paths.length)) {
return done(action, partial);
}
const paths = await resolvePaths(dir, body);
const untracked = paths.filter((p) => p.code === "??").map((p) => p.path);
// A rename's ORIGINAL path rides along: restoring only the new one would
@@ -431,7 +438,7 @@ async function runGitAction(dir, action, body = {}) {
const tracked = paths
.filter((p) => p.code !== "??")
.flatMap((p) => (p.from && p.code.includes("R") ? [p.path, p.from] : [p.path]));
const out = [];
const out = partial ? [partial] : [];
// Untracked files are deleted (git has no copy to restore); tracked ones
// go back to HEAD in both gutters, so a staged-then-edited file lands in
// one state rather than half-reverted.
@@ -825,18 +832,11 @@ function hasHunks(body) {
const MAX_PARTIAL_FILES = 300;
/**
* JetBrains' partial commit: whole files (`paths`) plus, per file in `hunks`,
* only the ticked change blocks — `{"<path>": [{hunk: "@@ … @@", blocks: [0]}]}`,
* the hunks named as `stage-hunk` names them, at `context`. Built in a
* TEMPORARY index (HEAD + the whole files + one patch per partial file from
* `selectionPatch`, git's own fresh text, never the request's) and committed
* from it, so the real index and the working tree are never in the way; after
* the commit the real index entries of exactly those paths are reset to the
* new HEAD, the rest of each partial file left as an unstaged change. Same
* path rules as `commitPaths`; a partial file must be tracked (EUNTRACKED)
* and not a rename, and a hunk the tree no longer has is EHUNKGONE.
* `body.hunks` checked for shape, and its files resolved as changed paths
* that may be taken in part: tracked (EUNTRACKED), not a rename, not in
* conflict, and not also named whole in `body.paths`.
*/
async function commitSelection(dir, args, body) {
async function partialPicks(dir, body, verb) {
const picks = Object.entries(body.hunks);
if (picks.length > MAX_PARTIAL_FILES) throw bad("EBADPATH", "too many files");
for (const [, list] of picks) {
@@ -858,12 +858,54 @@ async function commitSelection(dir, args, body) {
const conflicted = [...whole, ...part].find((p) => CONFLICT_CODES.has(p.code));
if (conflicted) throw bad("ECONFLICTS", `${conflicted.path} is in conflict: resolve it first`);
for (const p of part) {
if (p.code === "??") throw bad("EUNTRACKED", `${p.path} is untracked: commit it whole`);
if (p.code.includes("R")) throw bad("EBADPATH", `${p.path} is a rename: commit it whole`);
if (p.code === "??") throw bad("EUNTRACKED", `${p.path} is untracked: ${verb} it whole`);
if (p.code.includes("R")) throw bad("EBADPATH", `${p.path} is a rename: ${verb} it whole`);
if (whole.some((w) => w.path === p.path)) {
throw bad("EBADPATH", `${p.path} is named both whole and in part`);
}
}
return { picks, whole, part };
}
/**
* The partial discard (#87): per file in `hunks`, only the ticked change
* blocks are undone off the working tree — `selectionPatch` in reverse, git's
* own fresh text, never the request's — exactly what one `discard-hunk` per
* block would do, in one step. Every patch is built before the first is
* applied, so a stale selection refuses (EHUNKGONE) with nothing undone.
*/
async function discardSelection(dir, body) {
const { picks } = await partialPicks(dir, body, "discard");
const patches = [];
for (const [p, list] of picks) {
const patch = await selectionPatch(dir, p, body.context, list, true);
if (!patch)
throw bad("EHUNKGONE", `${p}: that change is no longer in the diff; refresh and retry`);
patches.push(patch);
}
// The patch names paths from the repo root, so it is applied there.
const top = (await git(dir, ["rev-parse", "--show-toplevel"])).stdout.trim();
const zero = Number(body.context) === 0 ? ["--unidiff-zero"] : [];
const out = [];
for (const patch of patches)
out.push(await git(top, ["apply", "-R", ...zero, "-"], { input: patch }));
return mergeResults(out);
}
/**
* JetBrains' partial commit: whole files (`paths`) plus, per file in `hunks`,
* only the ticked change blocks — `{"<path>": [{hunk: "@@ … @@", blocks: [0]}]}`,
* the hunks named as `stage-hunk` names them, at `context`. Built in a
* TEMPORARY index (HEAD + the whole files + one patch per partial file from
* `selectionPatch`, git's own fresh text, never the request's) and committed
* from it, so the real index and the working tree are never in the way; after
* the commit the real index entries of exactly those paths are reset to the
* new HEAD, the rest of each partial file left as an unstaged change. Same
* path rules as `commitPaths`; a partial file must be tracked (EUNTRACKED)
* and not a rename, and a hunk the tree no longer has is EHUNKGONE.
*/
async function commitSelection(dir, args, body) {
const { picks, whole, part } = await partialPicks(dir, body, "commit");
const wholePaths = whole.flatMap((p) =>
p.from && p.code.includes("R") ? [p.path, p.from] : [p.path]
);