mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(paths): one home-prefix helper, so labels abbreviate on both platforms
The rule "show ~/project rather than /home/<user>/project" had three implementations in the frontend, two of them platform-specific in opposite directions, so each looked correct to whoever wrote it. - The Run menu's Recent Sessions rows matched /home/<user>/ only. On macOS nothing was stripped, so every row spent its first ~19 characters on an identical /Users/<user>/ prefix and the left-to-right ellipsis removed the tail that identifies the row. That is #273, reported by @jordan8037310, who also traced why the menu's 250px cap made it worse: the width was chosen on the assumption the abbreviation had run. - The case-manage list matched /Users/<user> only, the mirror image, so on a Linux host no case path was ever abbreviated there. Unreported. Both now call _shortenHomePath(), which was already correct for both layouts and already used by the Resume list, Cmd+K, the desktop home rail and the phone overview. Its regex collapses to one alternation with a lookahead, so a path that is exactly $HOME renders "~" instead of being left raw, matching what the case-manage list used to do on macOS. test/home-path-abbreviation.test.ts pins the helper on both layouts and the rendered case-manage label, and fails if a fourth copy of the pattern appears in src/web/public. The Run-menu guard counts helper calls rather than pinning a source line, so it survives the row restructure in #274. test/run-mode-ui.test.ts gains a _shortenHomePath stub: its harness loads session-ui.js without terminal-ui.js, which the real app never does. Verified against an isolated instance with 27 real cases and 50 history rows: 27 of 27 case paths and 17 of 20 Run menu rows abbreviate, the other 3 are /tmp paths that correctly stay raw, tooltips keep the full path, no page errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -489,7 +489,11 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
const date = new Date(s.lastModified);
|
const date = new Date(s.lastModified);
|
||||||
const timeStr = date.toLocaleDateString('en', { month: 'short', day: 'numeric' })
|
const timeStr = date.toLocaleDateString('en', { month: 'short', day: 'numeric' })
|
||||||
+ ' ' + date.toLocaleTimeString('en', { hour: '2-digit', minute: '2-digit', hour12: false });
|
+ ' ' + date.toLocaleTimeString('en', { hour: '2-digit', minute: '2-digit', hour12: false });
|
||||||
const shortDir = s.workingDir.replace(/^\/home\/[^/]+\//, '~/');
|
// Shared helper, not a local regex: the copy that used to live here
|
||||||
|
// matched `/home/<user>/` only, so on macOS every row rendered the same
|
||||||
|
// unabbreviated `/Users/<user>/…` prefix and ellipsized away the tail
|
||||||
|
// that identifies it (#273).
|
||||||
|
const shortDir = this._shortenHomePath(s.workingDir);
|
||||||
|
|
||||||
const btn = document.createElement('button');
|
const btn = document.createElement('button');
|
||||||
btn.className = 'run-mode-option';
|
btn.className = 'run-mode-option';
|
||||||
@@ -2676,7 +2680,9 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
cases.forEach((c, idx) => {
|
cases.forEach((c, idx) => {
|
||||||
const isFirst = idx === 0;
|
const isFirst = idx === 0;
|
||||||
const isLast = idx === cases.length - 1;
|
const isLast = idx === cases.length - 1;
|
||||||
const pathDisplay = c.path ? c.path.replace(/^\/Users\/[^/]+/, '~') : '';
|
// Was `/Users/<user>` only, the mirror image of the Run menu's bug: every
|
||||||
|
// case path on a Linux host rendered in full, unabbreviated.
|
||||||
|
const pathDisplay = c.path ? this._shortenHomePath(c.path) : '';
|
||||||
html += `
|
html += `
|
||||||
<div class="case-manage-item" data-case="${escapeHtml(c.name)}">
|
<div class="case-manage-item" data-case="${escapeHtml(c.name)}">
|
||||||
<div class="case-manage-info">
|
<div class="case-manage-info">
|
||||||
|
|||||||
@@ -1610,11 +1610,21 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
return workingDir.split('/').pop() || workingDir;
|
return workingDir.split('/').pop() || workingDir;
|
||||||
},
|
},
|
||||||
|
|
||||||
/** Normalize home prefixes to "~/" on both Linux and macOS */
|
/**
|
||||||
|
* Normalize a home prefix to "~" on both Linux (`/home/<user>`) and macOS
|
||||||
|
* (`/Users/<user>`). The lookahead lets the home directory ITSELF match, so a
|
||||||
|
* path that is exactly `$HOME` renders "~" instead of being left raw.
|
||||||
|
*
|
||||||
|
* This is the only place that pattern belongs. Two hand-rolled copies had
|
||||||
|
* drifted, each broken on the platform its author was not using: the Run
|
||||||
|
* menu's matched `/home/` only, so on macOS nothing was stripped and every
|
||||||
|
* Recent Sessions row spent its first ~19 characters on an identical
|
||||||
|
* `/Users/<user>/` prefix (#273); the case-manage list's matched `/Users/`
|
||||||
|
* only, so no Linux path was ever abbreviated there. Route new path labels
|
||||||
|
* through here rather than writing a third copy.
|
||||||
|
*/
|
||||||
_shortenHomePath(p) {
|
_shortenHomePath(p) {
|
||||||
return (p || '')
|
return (p || '').replace(/^\/(?:home|Users)\/[^/]+(?=\/|$)/, '~');
|
||||||
.replace(/^\/home\/[^/]+\//, '~/')
|
|
||||||
.replace(/^\/Users\/[^/]+\//, '~/');
|
|
||||||
},
|
},
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -0,0 +1,166 @@
|
|||||||
|
/**
|
||||||
|
* @fileoverview Issue #273 and its mirror image: abbreviating `$HOME` in path labels.
|
||||||
|
*
|
||||||
|
* The rule ("show `~/project` rather than `/home/<user>/project`") had three
|
||||||
|
* implementations in the frontend, and two of them were platform-specific in
|
||||||
|
* opposite directions, so each looked correct to whoever wrote it:
|
||||||
|
*
|
||||||
|
* - the Run menu's Recent Sessions rows matched `/home/<user>/` only, so on
|
||||||
|
* macOS nothing was stripped, every row spent its first ~19 characters on an
|
||||||
|
* identical `/Users/<user>/` prefix, and the left-to-right ellipsis removed
|
||||||
|
* the tail that identifies the row (#273),
|
||||||
|
* - the case-manage list matched `/Users/<user>` only, so on a Linux host no
|
||||||
|
* case path was ever abbreviated at all.
|
||||||
|
*
|
||||||
|
* Both now call `_shortenHomePath()`, which is pinned here for both layouts, and
|
||||||
|
* a static guard fails if a fourth copy of the pattern appears.
|
||||||
|
*
|
||||||
|
* Loaded via `vm` against a stub CodemanApp with a fake DOM, same harness as
|
||||||
|
* history-list-controls.test.ts. Port: none (no browser, no server).
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { readdirSync, readFileSync } from 'node:fs';
|
||||||
|
import { resolve } from 'node:path';
|
||||||
|
import vm from 'node:vm';
|
||||||
|
import { describe, expect, it, vi } from 'vitest';
|
||||||
|
|
||||||
|
/* eslint-disable @typescript-eslint/no-explicit-any */
|
||||||
|
|
||||||
|
const PUBLIC = resolve(import.meta.dirname, '../src/web/public');
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The container the vm's `document.getElementById` resolves for the case list.
|
||||||
|
* Swapped per test: the closure lives in THIS realm, so the shipping code inside
|
||||||
|
* the vm reads whatever the current test installed.
|
||||||
|
*/
|
||||||
|
let currentCaseList: { innerHTML: string } | null = null;
|
||||||
|
|
||||||
|
function loadTerminalUiPrototype(): Record<string, any> {
|
||||||
|
const source = readFileSync(resolve(PUBLIC, 'terminal-ui.js'), 'utf8');
|
||||||
|
const context = vm.createContext({
|
||||||
|
console,
|
||||||
|
CodemanApp: class CodemanApp {},
|
||||||
|
setInterval: vi.fn(),
|
||||||
|
clearInterval: vi.fn(),
|
||||||
|
setTimeout,
|
||||||
|
clearTimeout,
|
||||||
|
requestAnimationFrame: vi.fn(),
|
||||||
|
document: { addEventListener: vi.fn(), getElementById: () => null, createElement: () => ({}) },
|
||||||
|
window: { addEventListener: vi.fn(), removeEventListener: vi.fn() },
|
||||||
|
});
|
||||||
|
vm.runInContext(`${source}\nglobalThis.__proto = CodemanApp.prototype;`, context);
|
||||||
|
return (context as unknown as { __proto: Record<string, any> }).__proto;
|
||||||
|
}
|
||||||
|
|
||||||
|
function loadSessionUiPrototype(): Record<string, any> {
|
||||||
|
const source = readFileSync(resolve(PUBLIC, 'session-ui.js'), 'utf8');
|
||||||
|
const context = vm.createContext({
|
||||||
|
console,
|
||||||
|
CodemanApp: class CodemanApp {},
|
||||||
|
VoiceInput: {},
|
||||||
|
escapeHtml: (t: unknown) => String(t ?? ''),
|
||||||
|
setTimeout,
|
||||||
|
clearTimeout,
|
||||||
|
localStorage: { getItem: () => null, setItem: () => {} },
|
||||||
|
document: { getElementById: (id: string) => (id === 'caseManageList' ? currentCaseList : null) },
|
||||||
|
window: { addEventListener: vi.fn() },
|
||||||
|
});
|
||||||
|
vm.runInContext(`${source}\nglobalThis.__proto = CodemanApp.prototype;`, context);
|
||||||
|
return (context as unknown as { __proto: Record<string, any> }).__proto;
|
||||||
|
}
|
||||||
|
|
||||||
|
const terminalProto = loadTerminalUiPrototype();
|
||||||
|
const sessionProto = loadSessionUiPrototype();
|
||||||
|
const shorten = (p: unknown) => terminalProto._shortenHomePath.call(terminalProto, p);
|
||||||
|
|
||||||
|
describe('_shortenHomePath', () => {
|
||||||
|
it('abbreviates the Linux home prefix', () => {
|
||||||
|
expect(shorten('/home/arkon/default/claudeman')).toBe('~/default/claudeman');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('abbreviates the macOS home prefix, which the Run menu never did (#273)', () => {
|
||||||
|
expect(shorten('/Users/jordanryan/code/facet/facet-agency-ops')).toBe('~/code/facet/facet-agency-ops');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('abbreviates the home directory itself, not only paths below it', () => {
|
||||||
|
// The case-manage list's old regex had no trailing slash and did collapse
|
||||||
|
// this to "~"; keep that, or a case whose path IS $HOME would regress.
|
||||||
|
expect(shorten('/home/arkon')).toBe('~');
|
||||||
|
expect(shorten('/Users/jordanryan')).toBe('~');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves paths that only look like a home prefix alone', () => {
|
||||||
|
expect(shorten('/homer/bob/x')).toBe('/homer/bob/x');
|
||||||
|
expect(shorten('/Userspace/bob/x')).toBe('/Userspace/bob/x');
|
||||||
|
expect(shorten('/home')).toBe('/home');
|
||||||
|
expect(shorten('/mnt/d/work')).toBe('/mnt/d/work');
|
||||||
|
expect(shorten('/opt/codeman')).toBe('/opt/codeman');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('replaces only the leading occurrence', () => {
|
||||||
|
expect(shorten('/home/arkon/home/bob/x')).toBe('~/home/bob/x');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('tolerates empty and missing input', () => {
|
||||||
|
expect(shorten('')).toBe('');
|
||||||
|
expect(shorten(undefined)).toBe('');
|
||||||
|
expect(shorten(null)).toBe('');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('renderCaseManageList path labels', () => {
|
||||||
|
function render(cases: Array<{ name: string; path: string; location?: string }>): string {
|
||||||
|
currentCaseList = { innerHTML: '' };
|
||||||
|
const app: any = {
|
||||||
|
cases,
|
||||||
|
_shortenHomePath: terminalProto._shortenHomePath,
|
||||||
|
renderCaseManageList: sessionProto.renderCaseManageList,
|
||||||
|
};
|
||||||
|
app.renderCaseManageList();
|
||||||
|
const html = currentCaseList.innerHTML;
|
||||||
|
currentCaseList = null;
|
||||||
|
return html;
|
||||||
|
}
|
||||||
|
|
||||||
|
it('abbreviates a Linux case path (the mirror of #273)', () => {
|
||||||
|
const html = render([{ name: 'demo', path: '/home/arkon/codeman-cases/demo' }]);
|
||||||
|
expect(html).toContain('~/codeman-cases/demo');
|
||||||
|
expect(html).not.toContain('/home/arkon/codeman-cases/demo');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still abbreviates a macOS case path', () => {
|
||||||
|
const html = render([{ name: 'demo', path: '/Users/jordanryan/codeman-cases/demo' }]);
|
||||||
|
expect(html).toContain('~/codeman-cases/demo');
|
||||||
|
expect(html).not.toContain('/Users/jordanryan/codeman-cases/demo');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('renders the row when a case has no path at all', () => {
|
||||||
|
const html = render([{ name: 'demo', path: '' }]);
|
||||||
|
expect(html).toContain('demo');
|
||||||
|
expect(html).toContain('class="case-manage-path"');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('single implementation of the home-prefix rule', () => {
|
||||||
|
/** Every top-level frontend module (vendor/ and subdirs are not ours). */
|
||||||
|
const sources = readdirSync(PUBLIC)
|
||||||
|
.filter((name) => name.endsWith('.js'))
|
||||||
|
.map((name) => ({ name, text: readFileSync(resolve(PUBLIC, name), 'utf8') }));
|
||||||
|
|
||||||
|
it('has exactly one home-prefix regex, in terminal-ui.js', () => {
|
||||||
|
// Any regex literal anchored at a home root. Three of these had drifted
|
||||||
|
// apart; a fourth would drift the same way.
|
||||||
|
const pattern = /\/\^\\\/(?:\(\?:home\|Users\)|home|Users)\\\//g;
|
||||||
|
const hits = sources.flatMap(({ name, text }) => (text.match(pattern) ?? []).map(() => name));
|
||||||
|
expect(hits).toEqual(['terminal-ui.js']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('routes both session-ui path labels through the helper', () => {
|
||||||
|
// Deliberately counts calls rather than pinning source lines: the Run menu
|
||||||
|
// row is being restructured in #274, and this guard should survive that as
|
||||||
|
// long as the label still goes through the helper.
|
||||||
|
const sessionUi = sources.find((s) => s.name === 'session-ui.js')!.text;
|
||||||
|
const calls = sessionUi.match(/this\._shortenHomePath\(/g) ?? [];
|
||||||
|
expect(calls.length).toBeGreaterThanOrEqual(2);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -760,6 +760,11 @@ describe('case selector refresh', () => {
|
|||||||
];
|
];
|
||||||
app.showToast = vi.fn();
|
app.showToast = vi.fn();
|
||||||
|
|
||||||
|
// deleteCase re-renders the case-manage list, whose path label goes through
|
||||||
|
// _shortenHomePath. That method lives in terminal-ui.js, which this harness
|
||||||
|
// does not load (the real app always has it: load order 7 before 12).
|
||||||
|
app._shortenHomePath = (p: string) => p;
|
||||||
|
|
||||||
await app.deleteCase('deleted-case');
|
await app.deleteCase('deleted-case');
|
||||||
|
|
||||||
expect(quickStartCase.blur).toHaveBeenCalled();
|
expect(quickStartCase.blur).toHaveBeenCalled();
|
||||||
|
|||||||
Reference in New Issue
Block a user