Skip to content

Commit 8d92fa9

Browse files
jonathanlabjonathanlab
committed
bug fixes
1 parent 386c52c commit 8d92fa9

8 files changed

Lines changed: 149 additions & 81 deletions

File tree

‎apps/cli/src/commands/log.ts‎

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -166,19 +166,26 @@ function renderEnhancedOutput(
166166
let currentIsTrunk = false;
167167
let currentIsForkPoint = false; // Immutable commit included only for graph connectivity
168168
let currentIsBehindTrunk = false; // Mutable commit whose parent is not current trunk
169+
let currentIsWorkingCopy = false; // Whether this is the @ commit
170+
let _currentIsEmpty = false; // Whether the change has no file modifications
169171
let pendingHints: string[] = []; // Buffer hints to output after COMMIT
170172

171-
// Check if WC parent is a tracked bookmark (for modify hint)
172-
// We only show "arr modify" if WC is on top of a tracked change
173-
const wcParentBookmark: string | null = null;
173+
// Find WC parent bookmark for modify hint by looking at the next CHANGE after WC
174+
let wcParentBookmark: string | null = null;
175+
let foundWC = false;
174176
for (const line of lines) {
175-
if (line.includes("HINT:empty")) {
176-
// WC is empty - check if parent is a tracked bookmark by looking at graph structure
177-
// The parent in jj graph is indicated by the line connecting @ to the next commit
178-
// But we can't reliably parse this from output, so check tracked bookmarks
179-
// If there's exactly one tracked bookmark that's a direct parent of @, use it
180-
// For now, we'll be conservative and not show modify hint unless we're certain
181-
// TODO: Query jj for @- to get actual parent
177+
if (line.includes("@") && line.includes("CHANGE:")) {
178+
foundWC = true;
179+
continue;
180+
}
181+
if (foundWC && line.includes("CHANGE:")) {
182+
// This is the first change after WC - check if it has a tracked bookmark
183+
const match = line.match(/CHANGE:[^|]+\|[^|]*\|[^|]*\|[^|]*\|([^|]*)\|/);
184+
if (match) {
185+
const bookmarks = parseBookmarks(match[1]);
186+
wcParentBookmark =
187+
bookmarks.find((b) => trackedBookmarks.includes(b)) || null;
188+
}
182189
break;
183190
}
184191
}
@@ -223,6 +230,7 @@ function renderEnhancedOutput(
223230
const isEmpty = emptyFlag === "1";
224231
const isImmutable = immutableFlag === "1";
225232
const hasConflict = conflictFlag === "1";
233+
const isWorkingCopy = graphPrefix.includes("@");
226234

227235
// Update context for subsequent lines (TIME, PR, COMMIT)
228236
currentBookmark =
@@ -235,6 +243,8 @@ function renderEnhancedOutput(
235243
// Fork point: immutable commit that's not trunk (included for graph connectivity)
236244
currentIsForkPoint = isImmutable && !isTrunk;
237245
currentIsBehindTrunk = behindTrunkChanges.has(changeId);
246+
currentIsWorkingCopy = isWorkingCopy;
247+
_currentIsEmpty = isEmpty;
238248

239249
// Skip rendering fork points - just keep graph lines
240250
if (currentIsForkPoint) {
@@ -249,7 +259,7 @@ function renderEnhancedOutput(
249259
// Replace the marker in graphPrefix with our styled version
250260
// jj uses: @ for WC, ○ for mutable, ◆ for immutable
251261
let styledPrefix = graphPrefix;
252-
if (graphPrefix.includes("@")) {
262+
if (isWorkingCopy) {
253263
styledPrefix = graphPrefix.replace("@", green("◉"));
254264
} else if (graphPrefix.includes("◆")) {
255265
styledPrefix = graphPrefix.replace("◆", "◯");
@@ -258,8 +268,8 @@ function renderEnhancedOutput(
258268
}
259269

260270
// Build the label
261-
if (isEmpty && !description && !isImmutable) {
262-
// Empty WC
271+
if (isWorkingCopy && !currentBookmark) {
272+
// Working copy without a bookmark - show "(working copy)"
263273
output.push(`${styledPrefix}${blue("(working copy)")}`);
264274
} else if (isTrunk) {
265275
output.push(`${styledPrefix}${blue(trunkName)}`);
@@ -298,20 +308,8 @@ function renderEnhancedOutput(
298308
}
299309

300310
case "HINT:": {
301-
if (data === "empty") {
302-
// Buffer hints to output after COMMIT line
303-
// Use a clean "│ " prefix, not the graph prefix which may have ~ terminators
304-
const hintPrefix = "│ ";
305-
pendingHints.push(
306-
`${hintPrefix}${arr(COMMANDS.create)} ${dim('"message"')} ${dim("to save as new change")}`,
307-
);
308-
// Only show modify hint if WC parent is a tracked bookmark
309-
if (wcParentBookmark) {
310-
pendingHints.push(
311-
`${hintPrefix}${arr(COMMANDS.modify)} ${dim(`to update ${wcParentBookmark}`)}`,
312-
);
313-
}
314-
}
311+
// Hints are now handled in COMMIT case for all WC states
312+
// This case is kept for potential future use
315313
break;
316314
}
317315

@@ -370,6 +368,20 @@ function renderEnhancedOutput(
370368
output.push(
371369
`${prefix}${commitIdFormatted} ${dim(`- ${description || "(no description)"}`)}`,
372370
);
371+
372+
// Add hints for WC without a bookmark (whether empty or with changes)
373+
if (currentIsWorkingCopy && !currentBookmark) {
374+
const hintPrefix = "│ ";
375+
pendingHints.push(
376+
`${hintPrefix}${arr(COMMANDS.create)} ${dim('"message"')} ${dim("to save as new change")}`,
377+
);
378+
if (wcParentBookmark) {
379+
pendingHints.push(
380+
`${hintPrefix}${arr(COMMANDS.modify)} ${dim(`to update ${wcParentBookmark}`)}`,
381+
);
382+
}
383+
}
384+
373385
// Output any pending hints after commit
374386
if (pendingHints.length > 0) {
375387
for (const hint of pendingHints) {

‎apps/cli/src/utils/output.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export function formatSuccess(message: string): string {
8080
*
8181
* Messages:
8282
* - editing: "Editing branch-name"
83-
* - on-top: "Working on top of branch-name"
83+
* - on-top: "Now above branch-name"
8484
* - on-trunk: "Starting fresh on main"
8585
*/
8686
export function printNavResult(nav: NavigationResult): void {
@@ -91,7 +91,7 @@ export function printNavResult(nav: NavigationResult): void {
9191
console.log(`Editing ${green(label)}`);
9292
break;
9393
case "on-top":
94-
console.log(`Working on top of ${green(label)}`);
94+
console.log(`Now above ${green(label)}`);
9595
break;
9696
case "on-trunk":
9797
console.log(`Starting fresh on ${cyan(label)}`);

‎packages/core/src/commands/create.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { resolveBookmarkConflict } from "../bookmark-utils";
22
import type { Engine } from "../engine";
33
import { ensureBookmark, runJJ, status } from "../jj";
4-
import { ok, type Result } from "../result";
4+
import { createError, err, ok, type Result } from "../result";
55
import { datePrefixedLabel } from "../slugify";
66
import type { Command } from "./types";
77

@@ -39,6 +39,18 @@ export async function create(
3939
if (!statusResult.ok) return statusResult;
4040

4141
const wc = statusResult.value.workingCopy;
42+
const hasChanges = statusResult.value.modifiedFiles.length > 0;
43+
44+
// Don't allow creating empty changes
45+
if (!hasChanges) {
46+
return err(
47+
createError(
48+
"EMPTY_CHANGE",
49+
"No file changes to create. Make some changes first.",
50+
),
51+
);
52+
}
53+
4254
let createdChangeId: string;
4355

4456
if (wc.description.trim() !== "") {

‎packages/core/src/commands/restack.ts‎

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,52 +18,64 @@ interface RestackOptions {
1818
}
1919

2020
/**
21-
* Find tracked bookmarks that are behind trunk (not based on current trunk tip).
21+
* Find root bookmarks that are behind trunk.
22+
* Roots are tracked bookmarks whose parent is NOT another tracked bookmark.
23+
* We only rebase roots - descendants will follow automatically.
2224
*/
23-
async function getBookmarksBehindTrunk(
25+
async function getRootBookmarksBehindTrunk(
2426
trackedBookmarks: string[],
2527
): Promise<Result<string[]>> {
2628
if (trackedBookmarks.length === 0) {
2729
return ok([]);
2830
}
2931

30-
const behindBookmarks: string[] = [];
31-
32-
for (const bookmark of trackedBookmarks) {
33-
// Check if this bookmark exists and is not a descendant of trunk
34-
const result = await runJJ([
35-
"log",
36-
"-r",
37-
`bookmarks(exact:"${bookmark}") & mutable() ~ trunk()::`,
38-
"--no-graph",
39-
"-T",
40-
`change_id ++ "\\n"`,
41-
]);
42-
43-
if (result.ok && result.value.stdout.trim()) {
44-
behindBookmarks.push(bookmark);
45-
}
46-
}
47-
48-
return ok(behindBookmarks);
32+
const bookmarkRevsets = trackedBookmarks
33+
.map((b) => `bookmarks(exact:"${b}")`)
34+
.join(" | ");
35+
36+
// Find roots of tracked bookmarks that are behind trunk
37+
// roots(X) gives commits in X with no ancestors also in X
38+
// ~ trunk():: filters to only those not already on trunk
39+
const rootsRevset = `roots((${bookmarkRevsets}) & mutable()) ~ trunk()::`;
40+
41+
const result = await runJJ([
42+
"log",
43+
"-r",
44+
rootsRevset,
45+
"--no-graph",
46+
"-T",
47+
'local_bookmarks.map(|b| b.name()).join(",") ++ "\\n"',
48+
]);
49+
50+
if (!result.ok) return result;
51+
52+
const rootBookmarks = result.value.stdout
53+
.trim()
54+
.split("\n")
55+
.filter((line) => line.trim())
56+
.flatMap((line) => line.split(",").filter((b) => b.trim()))
57+
.filter((b) => trackedBookmarks.includes(b));
58+
59+
return ok(rootBookmarks);
4960
}
5061

5162
/**
52-
* Rebase tracked bookmarks that are behind trunk.
63+
* Rebase root tracked bookmarks that are behind trunk.
64+
* Only rebases roots - descendants follow automatically.
5365
*/
5466
async function restackTracked(
5567
trackedBookmarks: string[],
5668
): Promise<Result<{ restacked: number }>> {
57-
const behindResult = await getBookmarksBehindTrunk(trackedBookmarks);
58-
if (!behindResult.ok) return behindResult;
69+
const rootsResult = await getRootBookmarksBehindTrunk(trackedBookmarks);
70+
if (!rootsResult.ok) return rootsResult;
5971

60-
const behind = behindResult.value;
61-
if (behind.length === 0) {
72+
const roots = rootsResult.value;
73+
if (roots.length === 0) {
6274
return ok({ restacked: 0 });
6375
}
6476

65-
// Rebase each behind bookmark onto trunk
66-
for (const bookmark of behind) {
77+
// Rebase each root bookmark onto trunk - descendants will follow
78+
for (const bookmark of roots) {
6779
const result = await runJJWithMutableConfigVoid([
6880
"rebase",
6981
"-b",
@@ -74,7 +86,7 @@ async function restackTracked(
7486
if (!result.ok) return result;
7587
}
7688

77-
return ok({ restacked: behind.length });
89+
return ok({ restacked: roots.length });
7890
}
7991

8092
/**

‎packages/core/src/commands/status.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,11 @@ export async function status(): Promise<Result<StatusResult>> {
137137
};
138138

139139
// Get diff stats for current change
140-
const statsResult = await getDiffStats("@");
140+
// If on a bookmark with origin, show diff since last push; otherwise show full commit diff
141+
const currentBookmark = wc.bookmarks[0];
142+
const statsResult = await getDiffStats("@", {
143+
fromBookmark: currentBookmark,
144+
});
141145
const stats = statsResult.ok ? statsResult.value : null;
142146

143147
return ok({ info, stats });

‎packages/core/src/commands/sync.ts‎

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -127,42 +127,69 @@ export async function sync(options: SyncOptions): Promise<Result<SyncResult>> {
127127
const trunk = await getTrunk();
128128
await runJJ(["bookmark", "set", trunk, "-r", `${trunk}@origin`]);
129129

130-
// Rebase WC onto new trunk if it's behind (empty WC on old trunk ancestor)
131-
await runJJWithMutableConfigVoid(["rebase", "-r", "@", "-d", "trunk()"]);
132-
133130
// Rebase only tracked bookmarks onto trunk (not all mutable commits)
134131
// This prevents rebasing unrelated orphaned commits from the repo history
135132
const trackedBookmarks = engine.getTrackedBookmarks();
136133
let rebaseOk = true;
137134
let rebaseError: string | undefined;
138135

139-
for (const bookmark of trackedBookmarks) {
140-
// Only rebase if bookmark exists and is mutable
141-
const checkResult = await runJJ([
136+
// Build revset for all tracked bookmarks
137+
if (trackedBookmarks.length > 0) {
138+
const bookmarkRevsets = trackedBookmarks
139+
.map((b) => `bookmarks(exact:"${b}")`)
140+
.join(" | ");
141+
142+
// Find roots of tracked bookmarks - those whose parent is NOT another tracked bookmark
143+
// roots(X) gives us commits in X that have no ancestors also in X
144+
const rootsRevset = `roots((${bookmarkRevsets}) & mutable())`;
145+
146+
const rootsResult = await runJJ([
142147
"log",
143148
"-r",
144-
`bookmarks(exact:"${bookmark}") & mutable()`,
149+
rootsRevset,
145150
"--no-graph",
146151
"-T",
147-
"change_id",
152+
'local_bookmarks.map(|b| b.name()).join(",") ++ "\\n"',
148153
]);
149154

150-
if (checkResult.ok && checkResult.value.stdout.trim()) {
151-
const result = await runJJWithMutableConfigVoid([
152-
"rebase",
153-
"-b",
154-
bookmark,
155-
"-d",
156-
"trunk()",
157-
]);
158-
if (!result.ok) {
159-
rebaseOk = false;
160-
rebaseError = result.error.message;
161-
break;
155+
if (rootsResult.ok) {
156+
const rootBookmarks = rootsResult.value.stdout
157+
.trim()
158+
.split("\n")
159+
.filter((line) => line.trim())
160+
.flatMap((line) => line.split(",").filter((b) => b.trim()));
161+
162+
// Only rebase root bookmarks - descendants will follow
163+
for (const bookmark of rootBookmarks) {
164+
if (!trackedBookmarks.includes(bookmark)) continue;
165+
166+
const result = await runJJWithMutableConfigVoid([
167+
"rebase",
168+
"-b",
169+
bookmark,
170+
"-d",
171+
"trunk()",
172+
]);
173+
if (!result.ok) {
174+
rebaseOk = false;
175+
rebaseError = result.error.message;
176+
break;
177+
}
162178
}
163179
}
164180
}
165181

182+
// Rebase WC onto trunk if it's not on a tracked bookmark
183+
// (If WC is on a tracked bookmark, it was already rebased above)
184+
const wcStatusResult = await status();
185+
if (wcStatusResult.ok) {
186+
const wcBookmarks = wcStatusResult.value.workingCopy.bookmarks;
187+
const wcOnTracked = wcBookmarks.some((b) => trackedBookmarks.includes(b));
188+
if (!wcOnTracked) {
189+
await runJJWithMutableConfigVoid(["rebase", "-r", "@", "-d", "trunk()"]);
190+
}
191+
}
192+
166193
// Check for conflicts
167194
let hasConflicts = false;
168195
if (rebaseOk) {

‎packages/core/src/jj/diff.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export async function getDiffStats(
3636
"diff",
3737
"--from",
3838
`${options.fromBookmark}@origin`,
39-
"-r",
39+
"--to",
4040
revision,
4141
"--stat",
4242
],

‎packages/core/src/result.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export type JJErrorCode =
3232
| "MERGE_BLOCKED"
3333
| "ALREADY_MERGED"
3434
| "NOT_FOUND"
35+
| "EMPTY_CHANGE"
3536
| "UNKNOWN";
3637

3738
export function createError(

0 commit comments

Comments
 (0)