Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion src/git/__tests__/git-service.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { GitService, GitError, binCommitTime } from '../git-service';
import { GitService, GitError, binCommitTime, buildAmendCommandStr } from '../git-service';

// Access private exec method via prototype for mocking
function mockExec(service: GitService, fn: (args: string[]) => Promise<string>) {
Expand Down Expand Up @@ -1300,3 +1300,35 @@ describe('GitService', () => {
});
});
});

describe('buildAmendCommandStr', () => {
const escape = (s: string) => `'${s.replace(/'/g, "'\\''")}'`;

it('returns --no-edit -m for single-line messages', () => {
const cmd = buildAmendCommandStr('hello world', escape);
expect(cmd).toBe("git commit --amend --no-edit -m 'hello world'");
});

it('returns printf pipeline for multi-line messages', () => {
const cmd = buildAmendCommandStr('subject\n\nbody line', escape);
expect(cmd).toContain('printf');
expect(cmd).toContain('git commit --amend -F -');
expect(cmd).toContain("'subject'");
expect(cmd).toContain("''");
expect(cmd).toContain("'body line'");
});

it('escapes single quotes in multi-line messages', () => {
const cmd = buildAmendCommandStr("it's\nbroken", escape);
expect(cmd).toContain("'it'\\''s'");
expect(cmd).toContain("'broken'");
});

it('handles message with only newlines (empty lines)', () => {
const cmd = buildAmendCommandStr('\n\n', escape);
expect(cmd).toContain('printf');
const parts = cmd.split(' ');
// Three empty strings after split
expect(parts.filter(p => p === "''").length).toBe(3);
});
});
30 changes: 30 additions & 0 deletions src/git/__tests__/integration/merge-rebase.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,19 @@ describe('GitService integration — merge / rebase / cherry-pick / revert', ()
expect(subject).toBe('shiny new subject');
});

it('reword preserves multi-line commit message via printf pipeline', async () => {
commit(repo.path, 'init');
const base = head(repo.path);
const c1 = commit(repo.path, 'old subject', { 'a.txt': 'A\n' });

await svc.interactiveRebase(base, [
{ action: 'reword', hash: c1, subject: 'old subject', message: 'new subject\n\nbody line one\nbody line two' },
]);

const fullMsg = runGit(repo.path, ['log', '-1', '--format=%B']).trim();
expect(fullMsg).toBe('new subject\n\nbody line one\nbody line two');
});

it('squash collapses commits and uses the provided final message', async () => {
commit(repo.path, 'init');
const base = head(repo.path);
Expand All @@ -546,6 +559,23 @@ describe('GitService integration — merge / rebase / cherry-pick / revert', ()
expect(subjects[0]).toBe('combined ABC');
});

it('squash uses multi-line combined message from printf pipeline', async () => {
commit(repo.path, 'init');
const base = head(repo.path);
const c1 = commit(repo.path, 'A', { 'a.txt': 'A\n' });
const c2 = commit(repo.path, 'B', { 'b.txt': 'B\n' });
const c3 = commit(repo.path, 'C', { 'c.txt': 'C\n' });

await svc.interactiveRebase(base, [
{ action: 'pick', hash: c1, subject: 'A', message: 'combined title\n\n- item one\n- item two' },
{ action: 'squash', hash: c2, subject: 'B' },
{ action: 'squash', hash: c3, subject: 'C' },
]);

const fullMsg = runGit(repo.path, ['log', '-1', '--format=%B']).trim();
expect(fullMsg).toBe('combined title\n\n- item one\n- item two');
});

it('fixup discards the squashed commits message (no exec amend)', async () => {
commit(repo.path, 'init');
const base = head(repo.path);
Expand Down
32 changes: 20 additions & 12 deletions src/git/git-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1223,13 +1223,11 @@ export class GitService {
private shellEscapeForExec(s: string): string {
return `'${s.replace(/'/g, "'\\''")}'`;
}

private buildMFlags(parts: string[]): string {
return parts
.map(p => p.trim())
.filter(Boolean)
.map(p => `-m ${this.shellEscapeForExec(p)}`)
.join(' ');
/** Build amend command for interactive rebase exec lines.
* Single-line → `git commit --amend --no-edit -m 'msg'`.
* Multi-line → `printf '%s\n' ... | git commit --amend -F -` (POSIX, no temp files). */
private buildAmendCommand(message: string): string {
return buildAmendCommandStr(message, s => this.shellEscapeForExec(s));
}

/**
Expand Down Expand Up @@ -1260,7 +1258,6 @@ export class GitService {
);
}

// Write todo list to a temp file (avoids shell injection)
const isSquashLike = (a: string) => a === 'squash' || a === 'fixup';
const lines: string[] = [];
let i = 0;
Expand Down Expand Up @@ -1293,7 +1290,7 @@ export class GitService {
// Without this, fixup-only groups would silently discard the user's edited message, and
// squash groups would inherit git's default-editor combined message.
if (finalMessage && (messageChanged || userWantsReword)) {
lines.push(`exec git commit --amend --no-edit ${this.buildMFlags([finalMessage])}`);
lines.push(`exec ${this.buildAmendCommand(finalMessage)}`);
}
continue;
}
Expand All @@ -1303,12 +1300,11 @@ export class GitService {
const msg = (todo.message ?? todo.subject).trim();
lines.push(`pick ${todo.hash}`);
if (msg) {
lines.push(`exec git commit --amend --no-edit ${this.buildMFlags([msg])}`);
lines.push(`exec ${this.buildAmendCommand(msg)}`);
}
i++;
continue;
}

lines.push(`${todo.action} ${todo.hash}`);
i++;
}
Expand Down Expand Up @@ -1359,7 +1355,6 @@ export class GitService {
});
});
} finally {
// Clean up temp file
await unlink(todoFile).catch(() => {});
}
}
Expand Down Expand Up @@ -2091,3 +2086,16 @@ export class GitService {
}

}

/**
* Build a `git commit --amend` command for use in interactive rebase exec lines.
* Single-line messages → `--no-edit -m 'msg'`.
* Multi-line messages → POSIX printf pipeline to preserve newlines.
*/
export function buildAmendCommandStr(message: string, escape: (s: string) => string): string {
if (!message.includes('\n')) {
return `git commit --amend --no-edit -m ${escape(message)}`;
}
const parts = message.split('\n').map(l => escape(l));
return `printf '%s\\n' ${parts.join(' ')} | git commit --amend -F -`;
}
95 changes: 84 additions & 11 deletions webview-ui/src/components/rebase/InteractiveRebase.svelte
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script lang="ts">
import { onMount } from 'svelte';
import { onMount, tick } from 'svelte';
import { getVsCodeApi } from '../../lib/vscode-api';
import { t } from '../../lib/i18n/index.svelte';
import type { Commit } from '../../lib/types';
Expand All @@ -21,6 +21,7 @@
action: 'pick' | 'squash' | 'fixup' | 'reword' | 'edit' | 'drop';
hash: string;
subject: string;
body: string;
newMessage?: string;
}

Expand Down Expand Up @@ -56,15 +57,80 @@
return 'none';
}));

function autoresize(node: HTMLTextAreaElement) {
function resize() {
node.style.height = 'auto';
node.style.height = node.scrollHeight + 'px';
}
// Defer initial resize until after value binding is applied
tick().then(resize);
node.addEventListener('input', resize);
return {
destroy() { node.removeEventListener('input', resize); },
update() { tick().then(resize); },
};
}

function fullMessage(todo: TodoEntry): string {
return todo.body ? `${todo.subject}\n\n${todo.body}` : todo.subject;
}

function squashGroupMessage(startIndex: number): string {
const parts: string[] = [];
let i = startIndex;
parts.push(fullMessage(todos[i]));
i++;
while (i < todos.length && (todos[i].action === 'squash' || todos[i].action === 'fixup')) {
if (todos[i].action === 'squash') {
parts.push(fullMessage(todos[i]));
}
i++;
}
return parts.join('\n\n');
}

/** Fingerprint of a squash group — changes when members or their actions change. */
function squashFingerprint(startIndex: number): string {
let i = startIndex;
let fp = todos[i].hash;
i++;
while (i < todos.length && (todos[i].action === 'squash' || todos[i].action === 'fixup')) {
fp += '|' + todos[i].hash + ':' + todos[i].action;
i++;
}
return fp;
}

const groupPrints = $state<Record<number, string>>({});

$effect.pre(() => {
todos.forEach((todo, i) => {
let i = 0;
while (i < todos.length) {
const todo = todos[i];
const role = squashGroups[i];
if (role === 'squash-target' && todo.newMessage === undefined) {
todos[i].newMessage = todo.subject;
} else if (role !== 'squash-target' && todo.action !== 'reword' && todo.newMessage !== undefined) {
if (role === 'squash-target') {
const fp = squashFingerprint(i);
if (groupPrints[i] !== fp) {
// Group composition changed — reset to new combined message
groupPrints[i] = fp;
todos[i].newMessage = squashGroupMessage(i);
} else if (todo.newMessage === undefined) {
// First time — initialize
todos[i].newMessage = squashGroupMessage(i);
}
// Clear message on squash members
i++;
while (i < todos.length && (todos[i].action === 'squash' || todos[i].action === 'fixup')) {
todos[i].newMessage = undefined;
i++;
}
continue;
}
if (todo.action !== 'reword' && todo.newMessage !== undefined) {
todos[i].newMessage = undefined;
}
});
i++;
}
});

onMount(() => {
Expand All @@ -75,6 +141,7 @@
action: 'pick' as const,
hash: c.hash,
subject: c.subject,
body: c.body,
}));
initialOrder = msg.payload.commits.map((c: Commit) => c.hash);
loading = false;
Expand All @@ -97,7 +164,7 @@
function setAction(index: number, action: TodoEntry['action']) {
todos[index].action = action;
if (action === 'reword') {
todos[index].newMessage = todos[index].subject;
todos[index].newMessage = fullMessage(todos[index]);
} else {
todos[index].newMessage = undefined;
}
Expand Down Expand Up @@ -224,13 +291,14 @@
<div class="todo-content">
<span class="todo-hash">{todo.hash.substring(0, 7)}</span>
{#if todo.action === 'reword' || groupRole === 'squash-target'}
<input
<textarea
class="todo-message-input"
type="text"
rows="1"
placeholder={todo.action === 'reword' ? t('rebase.inlineDesc.reword') : t('rebase.inlineDesc.squash')}
use:tooltip={todo.subject}
use:autoresize
bind:value={todos[index].newMessage}
/>
></textarea>
{:else}
<span class="todo-subject truncate" class:dropped-text={todo.action === 'drop'} use:tooltip={todo.subject}>{todo.subject}</span>
{#if todo.action !== 'pick'}
Expand Down Expand Up @@ -536,13 +604,18 @@
.todo-message-input {
flex: 1;
min-width: 0;
padding: 1px 6px;
min-height: 28px;
max-height: 160px;
padding: 2px 6px;
background: var(--vscode-input-background, var(--bg-secondary));
border: 1px solid var(--vscode-input-border, var(--border-color));
border-radius: 3px;
color: var(--vscode-input-foreground, var(--text-primary));
font-size: inherit;
font-family: inherit;
line-height: 1.3;
resize: none;
overflow-y: auto;
outline: none;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,55 @@ describe('InteractiveRebase — action changes', () => {
expect(inputs.length).toBeGreaterThan(0);
});
});

it('pre-fills the full commit message (subject + body) when selecting reword on a commit with body', async () => {
const { container } = render(InteractiveRebase, baseProps);
deliverCommits([
commit({ hash: 'c1', subject: 'one', body: 'line two\nline three' }),
commit({ hash: 'c2', subject: 'two' }),
]);
await waitFor(() => container.querySelector('.todo-item'));
const badges = container.querySelectorAll<HTMLButtonElement>('.action-badge');
await fireEvent.click(badges[0]);
const opts = container.querySelectorAll<HTMLButtonElement>('.action-option');
await fireEvent.click(Array.from(opts).find(o => o.textContent?.toLowerCase().includes('reword'))!);
await waitFor(() => {
const textarea = container.querySelector<HTMLTextAreaElement>('.todo-message-input');
expect(textarea).not.toBeNull();
expect(textarea!.value).toContain('line two');
expect(textarea!.value).toContain('line three');
expect(textarea!.value).toContain('one');
});
});

it('shows combined subject+body for squash-target, excluding fixup messages', async () => {
const { container } = render(InteractiveRebase, baseProps);
deliverCommits([
commit({ hash: 'a', subject: 'target', body: 'target body' }),
commit({ hash: 'b', subject: 'squash msg', body: 'squash body' }),
commit({ hash: 'c', subject: 'fixup msg', body: 'fixup body' }),
]);
await waitFor(() => container.querySelector('.todo-item'));
const badges = container.querySelectorAll<HTMLButtonElement>('.action-badge');
// Squash commit b, fixup commit c
await fireEvent.click(badges[1]);
const squashOpts = container.querySelectorAll<HTMLButtonElement>('.action-option');
await fireEvent.click(Array.from(squashOpts).find(o => o.textContent?.toLowerCase().includes('squash'))!);
await fireEvent.click(badges[2]);
const fixupOpts = container.querySelectorAll<HTMLButtonElement>('.action-option');
await fireEvent.click(Array.from(fixupOpts).find(o => o.textContent?.toLowerCase().includes('fixup'))!);
await waitFor(() => {
const textarea = container.querySelector<HTMLTextAreaElement>('.todo-message-input');
expect(textarea).not.toBeNull();
expect(textarea!.value).toContain('target');
expect(textarea!.value).toContain('target body');
expect(textarea!.value).toContain('squash msg');
expect(textarea!.value).toContain('squash body');
// fixup messages should be excluded
expect(textarea!.value).not.toContain('fixup msg');
expect(textarea!.value).not.toContain('fixup body');
});
});
});

describe('InteractiveRebase — submit', () => {
Expand Down
Loading