mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-17 12:21:10 +00:00
refactor(cli): factor /diff column widths into a shared helper
Audit of the colorize commit found one real DRY hazard: DiffStatsDisplay and renderDiffModelText each independently re-derived addWidth / remWidth / statColumnWidth from the same row list. If anyone later changed one formula, the interactive Ink output and the non-interactive plain text would silently fall out of column alignment. Extract the computation into computeDiffColumnWidths() exported from diffCommand.ts; both renderers now call it. Adds a focused unit test of the contract (empty rows, widest non-binary row wins, binary rows are ignored, untracked text rows count). Drop a redundant `Omit<HistoryItemDiffStats, 'id'>` annotation since the type already has no id field.
This commit is contained in:
parent
8b277f66c3
commit
239fa7141e
4 changed files with 118 additions and 26 deletions
|
|
@ -6,7 +6,7 @@
|
|||
|
||||
import type { Mock } from 'vitest';
|
||||
import { vi, describe, it, expect, beforeEach } from 'vitest';
|
||||
import { diffCommand } from './diffCommand.js';
|
||||
import { computeDiffColumnWidths, diffCommand } from './diffCommand.js';
|
||||
import { type CommandContext } from './types.js';
|
||||
import { createMockCommandContext } from '../../test-utils/mockCommandContext.js';
|
||||
import { fetchGitDiff, type GitDiffResult } from '@qwen-code/qwen-code-core';
|
||||
|
|
@ -340,6 +340,83 @@ describe('diffCommand interactive mode', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('computeDiffColumnWidths', () => {
|
||||
// Direct contract test — both the Ink component and the plain-text
|
||||
// renderer call this helper, so its output binds their column alignment.
|
||||
// If anyone changes the formula, both paths must shift together.
|
||||
|
||||
it('reports min widths of 1 for an empty row list', () => {
|
||||
expect(computeDiffColumnWidths([])).toEqual({
|
||||
addWidth: 1,
|
||||
remWidth: 1,
|
||||
statColumnWidth: 5, // `+_ -_` with single-digit padding
|
||||
});
|
||||
});
|
||||
|
||||
it('sizes columns to the widest non-binary row', () => {
|
||||
const widths = computeDiffColumnWidths([
|
||||
{
|
||||
filename: 'a',
|
||||
added: 9999,
|
||||
removed: 5,
|
||||
isBinary: false,
|
||||
isUntracked: false,
|
||||
truncated: false,
|
||||
},
|
||||
{
|
||||
filename: 'b',
|
||||
added: 2,
|
||||
removed: 100,
|
||||
isBinary: false,
|
||||
isUntracked: false,
|
||||
truncated: false,
|
||||
},
|
||||
]);
|
||||
// 1 (`+`) + 4 (digits) + 1 (` `) + 1 (`-`) + 3 (digits) = 10
|
||||
expect(widths).toEqual({ addWidth: 4, remWidth: 3, statColumnWidth: 10 });
|
||||
});
|
||||
|
||||
it('ignores binary rows when computing widths', () => {
|
||||
// A binary row must not push the numeric column wider, otherwise the
|
||||
// `~` placeholder ends up padded to a column that no real number ever
|
||||
// occupies.
|
||||
const widths = computeDiffColumnWidths([
|
||||
{
|
||||
filename: 'a',
|
||||
added: 1,
|
||||
removed: 1,
|
||||
isBinary: false,
|
||||
isUntracked: false,
|
||||
truncated: false,
|
||||
},
|
||||
{
|
||||
filename: 'b.bin',
|
||||
isBinary: true,
|
||||
isUntracked: false,
|
||||
truncated: false,
|
||||
},
|
||||
]);
|
||||
expect(widths).toEqual({ addWidth: 1, remWidth: 1, statColumnWidth: 5 });
|
||||
});
|
||||
|
||||
it('counts untracked text rows in width calculation', () => {
|
||||
// Untracked rows render as `+N -0 filename (new)`; their `added`
|
||||
// value must be allowed to widen the column.
|
||||
const widths = computeDiffColumnWidths([
|
||||
{
|
||||
filename: 'fresh.log',
|
||||
added: 12345,
|
||||
removed: 0,
|
||||
isBinary: false,
|
||||
isUntracked: true,
|
||||
truncated: false,
|
||||
},
|
||||
]);
|
||||
expect(widths.addWidth).toBe(5);
|
||||
expect(widths.statColumnWidth).toBe(1 + 5 + 1 + 1 + 1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('diffCommand registration', () => {
|
||||
it('declares all execution modes so it works in non-interactive and ACP', () => {
|
||||
expect(diffCommand.supportedModes).toEqual([
|
||||
|
|
|
|||
|
|
@ -82,7 +82,7 @@ async function diffAction(
|
|||
// plain-text MessageActionReturn path so pipes, logs, and transports that
|
||||
// don't speak Ink still see legible output.
|
||||
if (context.executionMode === 'interactive') {
|
||||
const item: Omit<HistoryItemDiffStats, 'id'> = {
|
||||
const item: HistoryItemDiffStats = {
|
||||
type: MessageType.DIFF_STATS,
|
||||
model,
|
||||
};
|
||||
|
|
@ -136,6 +136,38 @@ function toRow(filename: string, s: PerFileStats): DiffRenderRow {
|
|||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Single source of truth for the per-row column layout. Used by both the
|
||||
* Ink component and the plain-text renderer so the two paths can never
|
||||
* silently disagree on alignment.
|
||||
*/
|
||||
export interface DiffColumnWidths {
|
||||
/** Digits in the widest non-binary `added` value (min 1). */
|
||||
addWidth: number;
|
||||
/** Digits in the widest non-binary `removed` value (min 1). */
|
||||
remWidth: number;
|
||||
/** Visual width of the `+X -Y` stat column, used to pad the binary `~`
|
||||
* marker so it lines up with the numeric rows. */
|
||||
statColumnWidth: number;
|
||||
}
|
||||
|
||||
export function computeDiffColumnWidths(
|
||||
rows: readonly DiffRenderRow[],
|
||||
): DiffColumnWidths {
|
||||
let maxAdded = 0;
|
||||
let maxRemoved = 0;
|
||||
for (const r of rows) {
|
||||
if (r.isBinary) continue;
|
||||
if ((r.added ?? 0) > maxAdded) maxAdded = r.added ?? 0;
|
||||
if ((r.removed ?? 0) > maxRemoved) maxRemoved = r.removed ?? 0;
|
||||
}
|
||||
const addWidth = String(maxAdded).length;
|
||||
const remWidth = String(maxRemoved).length;
|
||||
// `+` + addDigits + ' ' + `-` + remDigits.
|
||||
const statColumnWidth = 1 + addWidth + 1 + 1 + remWidth;
|
||||
return { addWidth, remWidth, statColumnWidth };
|
||||
}
|
||||
|
||||
/**
|
||||
* Plain-text rendering of a `DiffRenderModel`. Used in non-interactive / ACP
|
||||
* modes where no Ink renderer is available, and as the source of truth for
|
||||
|
|
@ -168,16 +200,7 @@ export function renderDiffModelText(model: DiffRenderModel): string {
|
|||
|
||||
function formatRowsText(rows: DiffRenderRow[]): string[] {
|
||||
if (rows.length === 0) return [];
|
||||
let maxAdded = 0;
|
||||
let maxRemoved = 0;
|
||||
for (const r of rows) {
|
||||
if (r.isBinary) continue;
|
||||
if ((r.added ?? 0) > maxAdded) maxAdded = r.added ?? 0;
|
||||
if ((r.removed ?? 0) > maxRemoved) maxRemoved = r.removed ?? 0;
|
||||
}
|
||||
const addWidth = String(maxAdded).length;
|
||||
const remWidth = String(maxRemoved).length;
|
||||
const statColumnWidth = 1 + addWidth + 1 + 1 + remWidth;
|
||||
const { addWidth, remWidth, statColumnWidth } = computeDiffColumnWidths(rows);
|
||||
|
||||
const out: string[] = [];
|
||||
for (const r of rows) {
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import type React from 'react';
|
|||
import { Box, Text } from 'ink';
|
||||
import { theme } from '../../semantic-colors.js';
|
||||
import type { DiffRenderModel, DiffRenderRow } from '../../types.js';
|
||||
import { computeDiffColumnWidths } from '../../commands/diffCommand.js';
|
||||
import { t } from '../../../i18n/index.js';
|
||||
|
||||
interface DiffStatsDisplayProps {
|
||||
|
|
@ -24,19 +25,10 @@ export const DiffStatsDisplay: React.FC<DiffStatsDisplayProps> = ({
|
|||
model,
|
||||
}) => {
|
||||
const { filesCount, linesAdded, linesRemoved, rows, hiddenCount } = model;
|
||||
|
||||
// Reproduce the numeric-column alignment of the text renderer so the
|
||||
// interactive and non-interactive outputs are visually interchangeable.
|
||||
let maxAdded = 0;
|
||||
let maxRemoved = 0;
|
||||
for (const r of rows) {
|
||||
if (r.isBinary) continue;
|
||||
if ((r.added ?? 0) > maxAdded) maxAdded = r.added ?? 0;
|
||||
if ((r.removed ?? 0) > maxRemoved) maxRemoved = r.removed ?? 0;
|
||||
}
|
||||
const addWidth = String(maxAdded).length;
|
||||
const remWidth = String(maxRemoved).length;
|
||||
const statColumnWidth = 1 + addWidth + 1 + 1 + remWidth;
|
||||
// Single source of truth shared with `renderDiffModelText`, so the
|
||||
// interactive Ink output and the non-interactive plain text never drift
|
||||
// out of column alignment.
|
||||
const { addWidth, remWidth, statColumnWidth } = computeDiffColumnWidths(rows);
|
||||
|
||||
const headerLabel =
|
||||
filesCount === 1
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
{
|
||||
"generatedAt": "2026-04-27T03:28:33.689Z",
|
||||
"generatedAt": "2026-04-27T03:39:04.133Z",
|
||||
"keys": [
|
||||
" Models: Qwen latest models\n",
|
||||
" qwen auth qwen-oauth - Authenticate with Qwen OAuth (discontinued)",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue