From 6d6dddd287fc30307d053492af51a57870cfbe9d Mon Sep 17 00:00:00 2001 From: Patrick Cheung Date: Fri, 25 Sep 2026 17:36:43 +0800 Subject: [PATCH] feat(graph): setting to hide stashes from the graph (#100) --- package.json | 5 ++ src/git/__tests__/git-service.extra.test.ts | 47 ++++++++++++++++ src/git/__tests__/git-service.test.ts | 51 ++++++++++++++++++ src/git/git-service.ts | 26 +++++++-- src/git/types.ts | 4 ++ src/panels/MainPanel.ts | 13 +++-- src/panels/__tests__/MainPanel.test.ts | 59 ++++++++++++++++++++- src/utils/__tests__/config.test.ts | 22 +++++++- src/utils/config.ts | 10 ++++ 9 files changed, 225 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index 8a1c2932..225fb104 100644 --- a/package.json +++ b/package.json @@ -439,6 +439,11 @@ "default": true, "markdownDescription": "Show a GPG/SSH signature status icon next to each commit in the graph. ⚠️ This verifies the signature of every commit in the log, which can slow down loading on very large repositories — turn it off if you notice slow graph loading. The Commit Details panel always shows signature status on demand regardless of this setting." }, + "gitGraphPlus.showStashes": { + "type": "boolean", + "default": true, + "markdownDescription": "Show stash entries in the commit graph and commit search results. Turn this off to hide stashes from the graph and search while keeping them available in the **Stashes** sidebar view." + }, "gitGraphPlus.branchBadgeBarThickness": { "type": "string", "enum": [ diff --git a/src/git/__tests__/git-service.extra.test.ts b/src/git/__tests__/git-service.extra.test.ts index 3b0ee028..d0ba6c28 100644 --- a/src/git/__tests__/git-service.extra.test.ts +++ b/src/git/__tests__/git-service.extra.test.ts @@ -574,6 +574,53 @@ describe('GitService — log() with stashes', () => { expect(commits.some(c => c.hash === 'c1')).toBe(true); expect(warn).toHaveBeenCalled(); }); + + it('resolves and inserts stashes by default (includeStashes omitted)', async () => { + const stashListSpy = vi.spyOn(service, 'stashList'); + mockExec(service, async (args) => { + if (args[0] === 'log' && !args.includes('--no-walk')) { + return logRecord('p1', 'p1', 'parent', ''); + } + if (args[0] === 'stash' && args[1] === 'list') { + return stashRecord(0, 'WIP', 'p1', 'sHash'); + } + if (args[0] === 'log' && args.includes('--no-walk')) { + return logRecord('sHash', 'sHash', 'wip', 'p1'); + } + if (args[0] === 'status') return ''; + return ''; + }); + + const commits = await service.log(); + expect(stashListSpy).toHaveBeenCalled(); + expect(commits.some(c => c.hash === 'sHash')).toBe(true); + }); + + it('skips stashList and stash insertion when includeStashes is false', async () => { + const calls: string[][] = []; + const stashListSpy = vi.spyOn(service, 'stashList'); + mockExec(service, async (args) => { + calls.push(args); + if (args[0] === 'log' && !args.includes('--no-walk')) { + return logRecord('p1', 'p1', 'parent', '') + logRecord('c1', 'c1', 'child', 'p1'); + } + if (args[0] === 'stash' && args[1] === 'list') { + return stashRecord(0, 'WIP', 'p1', 'sHash'); + } + if (args[0] === 'log' && args.includes('--no-walk')) { + return logRecord('sHash', 'sHash', 'wip', 'p1'); + } + if (args[0] === 'status') return ''; + return ''; + }); + + const commits = await service.log({ includeStashes: false }); + expect(stashListSpy).not.toHaveBeenCalled(); + expect(calls.some(a => a[0] === 'stash' && a[1] === 'list')).toBe(false); + expect(calls.some(a => a[0] === 'log' && a.includes('--no-walk'))).toBe(false); + expect(commits.some(c => c.hash === 'sHash')).toBe(false); + expect(commits.map(c => c.hash)).toContain('p1'); + }); }); describe('GitService — log() uncommitted porcelain branches', () => { diff --git a/src/git/__tests__/git-service.test.ts b/src/git/__tests__/git-service.test.ts index 923a6109..5ec06883 100644 --- a/src/git/__tests__/git-service.test.ts +++ b/src/git/__tests__/git-service.test.ts @@ -1124,6 +1124,57 @@ describe('GitService', () => { expect(args).toContain('--after=2024-01-01'); expect(args).toContain('--before=2024-12-31'); }); + + it('walks all refs (including refs/stash) by default', async () => { + const calls: string[][] = []; + (service as any).cachedRemoteNames = []; + (service as any).remoteNamesCacheTime = Date.now(); + mockExec(service, async (args) => { calls.push(args); return ''; }); + + await service.searchCommits('fix'); + const args = calls.find(a => a[0] === 'log')!; + expect(args).toContain('--all'); + expect(args).not.toContain('--exclude=refs/stash'); + }); + + it('excludes refs/stash when includeStashes is false', async () => { + const calls: string[][] = []; + (service as any).cachedRemoteNames = []; + (service as any).remoteNamesCacheTime = Date.now(); + mockExec(service, async (args) => { calls.push(args); return ''; }); + + await service.searchCommits('fix', { includeStashes: false }); + const args = calls.find(a => a[0] === 'log')!; + // --exclude must precede --all, which it scopes. + expect(args.indexOf('--exclude=refs/stash')).toBeGreaterThanOrEqual(0); + expect(args.indexOf('--exclude=refs/stash')).toBeLessThan(args.indexOf('--all')); + }); + }); + + describe('searchByFile includeStashes', () => { + it('walks all refs (including refs/stash) by default', async () => { + const calls: string[][] = []; + (service as any).cachedRemoteNames = []; + (service as any).remoteNamesCacheTime = Date.now(); + mockExec(service, async (args) => { calls.push(args); return ''; }); + + await service.searchByFile('src/app.ts'); + const args = calls.find(a => a[0] === 'log')!; + expect(args).toContain('--all'); + expect(args).not.toContain('--exclude=refs/stash'); + }); + + it('excludes refs/stash when includeStashes is false', async () => { + const calls: string[][] = []; + (service as any).cachedRemoteNames = []; + (service as any).remoteNamesCacheTime = Date.now(); + mockExec(service, async (args) => { calls.push(args); return ''; }); + + await service.searchByFile('src/app.ts', 100, { includeStashes: false }); + const args = calls.find(a => a[0] === 'log')!; + expect(args.indexOf('--exclude=refs/stash')).toBeGreaterThanOrEqual(0); + expect(args.indexOf('--exclude=refs/stash')).toBeLessThan(args.indexOf('--all')); + }); }); describe('lsTree', () => { diff --git a/src/git/git-service.ts b/src/git/git-service.ts index 71e65760..4a019ee1 100644 --- a/src/git/git-service.ts +++ b/src/git/git-service.ts @@ -526,7 +526,10 @@ export class GitService { // Resolve stashes before running the log: their base commits may need to be // added as extra walk start points (below). stashList() is deduped/cached, // so awaiting it here doesn't add a round-trip versus the old Promise.all. - const stashes = await this.stashList(); + // When includeStashes is false (gitGraphPlus.showStashes), skip the call + // and the insertion below so the graph never shows stash rows. + const includeStashes = options?.includeStashes !== false; + const stashes = includeStashes ? await this.stashList() : []; // Include each stash's base commit as an extra rev-list start point so git // walks the stash's ancestry down to where it rejoins the main history. @@ -1931,7 +1934,7 @@ export class GitService { // --- Phase 6: Search, Commit Template --- - async searchCommits(query: string, options?: { author?: string; after?: string; before?: string; limit?: number }): Promise { + async searchCommits(query: string, options?: { author?: string; after?: string; before?: string; limit?: number; includeStashes?: boolean }): Promise { // Defense-in-depth: reject control characters that could inject extra git // arguments. spawn() with explicit argv already prevents shell injection, // but a newline inside --grep=... lets a single user value carry multiple @@ -1949,11 +1952,17 @@ export class GitService { const args = [ 'log', '--format=%x01%x02%x03%H%x00%h%x00%an%x00%ae%x00%aI%x00%cn%x00%ce%x00%cI%x00%s%x00%P%x00%D%x00%b', - '--all', '--topo-order', `--max-count=${options?.limit ?? 200}`, ]; + if (options?.includeStashes === false) { + // `--exclude` scopes the `--all` below, dropping refs/stash (and the + // stash commits only reachable from it) from search results. + args.push('--exclude=refs/stash'); + } + args.push('--all'); + if (query) { args.push(`--grep=${query}`, '-i'); } @@ -1972,16 +1981,23 @@ export class GitService { return commits; } - async searchByFile(filePath: string, limit: number = 100): Promise { + async searchByFile(filePath: string, limit: number = 100, options?: { includeStashes?: boolean }): Promise { this.assertSafePath(filePath, 'log'); const args = [ 'log', '--format=%x01%x02%x03%H%x00%h%x00%an%x00%ae%x00%aI%x00%cn%x00%ce%x00%cI%x00%s%x00%P%x00%D%x00%b', + ]; + if (options?.includeStashes === false) { + // `--exclude` scopes the `--all` below, dropping refs/stash (and the + // stash commits only reachable from it) from search results. + args.push('--exclude=refs/stash'); + } + args.push( '--all', `--max-count=${limit}`, '--', filePath, - ]; + ); const [raw, remoteNames] = await Promise.all([this.exec(args), this.getRemoteNames()]); const commits = parseLog(raw, remoteNames); return commits; diff --git a/src/git/types.ts b/src/git/types.ts index feed7abb..1daca196 100644 --- a/src/git/types.ts +++ b/src/git/types.ts @@ -171,4 +171,8 @@ export interface LogOptions { * signatureStatus. Off by default — it forces GPG verification of every * commit in the log, which is slow on large repos. */ includeSignature?: boolean; + /** When false, skip stash resolution entirely: no `stash list` call, no + * stash rows, and no stash base commits added as extra walk start points. + * Defaults to true (stashes visible) when omitted. */ + includeStashes?: boolean; } diff --git a/src/panels/MainPanel.ts b/src/panels/MainPanel.ts index 5869905d..2e057d1e 100644 --- a/src/panels/MainPanel.ts +++ b/src/panels/MainPanel.ts @@ -5,7 +5,7 @@ import { GitService, GitError } from '../git/git-service'; import { formatGitError, isAuthFailure, transportFromRemoteUrl } from '../git/git-error-formatter'; import { splitUpstreamRef } from '../git/git-parser'; import { samePath } from '../utils/path'; -import { readTimeoutMs, readInitialCommitCount, readLoadMoreCommitCount, readInteractiveRebaseMode } from '../utils/config'; +import { readTimeoutMs, readInitialCommitCount, readLoadMoreCommitCount, readInteractiveRebaseMode, readShowStashes } from '../utils/config'; import { buildClassicRebaseCommand } from '../git/classic-rebase'; import { buildFullGraph } from '../git/git-graph-builder'; import { compileBranchColorRules, makeBranchColorResolver } from '../git/branch-color-resolver'; @@ -209,6 +209,10 @@ export class MainPanel { if (e.affectsConfiguration('gitGraphPlus.graphSortOrder')) { this.refreshAll(); } + if (e.affectsConfiguration('gitGraphPlus.showStashes')) { + // Toggling stash visibility changes the graph, so reload the log. + this.refreshAll(); + } if (e.affectsConfiguration('gitGraphPlus.locale')) { const localeSetting = vscode.workspace.getConfiguration('gitGraphPlus').get('locale', 'auto'); const locale = localeSetting === 'auto' ? (vscode.env.language || 'en') : localeSetting; @@ -416,7 +420,7 @@ export class MainPanel { this.isFirstGetLog = false; this.currentRemoteFilter = effectiveFilter; this.currentBranchFilter = effectiveBranchFilter; - const logPayload = { ...message.payload, remoteFilter: effectiveFilter, branches: effectiveBranchFilter, limit: requestedLimit + 1, sortOrder, includeSignature }; + const logPayload = { ...message.payload, remoteFilter: effectiveFilter, branches: effectiveBranchFilter, limit: requestedLimit + 1, sortOrder, includeSignature, includeStashes: readShowStashes() }; const seq = ++this.logSequence; const [allFetched, logBranches] = await Promise.all([ this.gitService.log(logPayload), @@ -1176,6 +1180,7 @@ export class MainPanel { author: message.payload.author, after: message.payload.after, before: message.payload.before, + includeStashes: readShowStashes(), }); if (seq !== this.searchSequence) break; this.post({ type: 'searchResults', payload: { commits: results, graph: [] } }); @@ -1190,7 +1195,7 @@ export class MainPanel { } case 'searchByFile': { const seq = ++this.searchSequence; - const results = await this.gitService.searchByFile(message.payload.file); + const results = await this.gitService.searchByFile(message.payload.file, undefined, { includeStashes: readShowStashes() }); if (seq !== this.searchSequence) break; this.post({ type: 'searchResults', payload: { commits: results, graph: [] } }); break; @@ -1790,7 +1795,7 @@ export class MainPanel { // repo-unrelated "demo"-looking graph. const remoteFilter = this.isFirstGetLog ? MainPanel.savedRemoteFilter : this.currentRemoteFilter; const branchFilter = this.isFirstGetLog ? MainPanel.savedBranchFilter : this.currentBranchFilter; - const logArgs = { limit: refreshLimit + 1, sortOrder, remoteFilter, branches: branchFilter, includeSignature }; + const logArgs = { limit: refreshLimit + 1, sortOrder, remoteFilter, branches: branchFilter, includeSignature, includeStashes: readShowStashes() }; const buildLogData = (allFetched: Awaited>, branches: Awaited>) => { const hasMore = allFetched.length > refreshLimit; diff --git a/src/panels/__tests__/MainPanel.test.ts b/src/panels/__tests__/MainPanel.test.ts index 76051ee3..33f5a7c6 100644 --- a/src/panels/__tests__/MainPanel.test.ts +++ b/src/panels/__tests__/MainPanel.test.ts @@ -31,9 +31,15 @@ const H = vi.hoisted(() => { setAuthRetryHandler: vi.fn(), setExtraEnv: vi.fn(), setDefaultTimeout: vi.fn(), + searchCommits: vi.fn(async () => []), + searchByFile: vi.fn(async () => []), }; return { git, + // Values returned by workspace.getConfiguration('gitGraphPlus').get(key, def); + // an absent key falls back to the caller-supplied default. + configValues: {} as Record, + configListener: null as null | ((e: { affectsConfiguration: (s: string) => boolean }) => void), messageHandler: null as null | ((m: unknown) => unknown), panel: null as null | { webview: { postMessage: ReturnType } }, repos: [] as Array<{ path: string; name: string; type: string }>, @@ -70,10 +76,15 @@ vi.mock('vscode', () => { showSaveDialog: vi.fn(async () => undefined), }, workspace: { - getConfiguration: () => ({ get: (_k: string, d?: unknown) => d }), + getConfiguration: () => ({ + get: (k: string, d?: unknown) => (H.configValues[k] === undefined ? d : H.configValues[k]), + }), getWorkspaceFolder: () => ({ uri: { fsPath: '/repo' } }), workspaceFolders: [{ uri: { fsPath: '/repo' } }], - onDidChangeConfiguration: () => ({ dispose() {} }), + onDidChangeConfiguration: (cb: (e: { affectsConfiguration: (s: string) => boolean }) => void) => { + H.configListener = cb; + return { dispose() {} }; + }, fs: { writeFile: vi.fn(async () => {}) }, }, commands: { executeCommand: vi.fn() }, @@ -127,6 +138,10 @@ beforeEach(() => { H.git.showCommitDiff.mockResolvedValue([]); H.git.fileExistsAtRef.mockResolvedValue(true); H.git.getEmptyTreeRef.mockResolvedValue('4b825dc642cb6eb9a060e54bf8d69288fbee4904'); + H.git.searchCommits.mockResolvedValue([]); + H.git.searchByFile.mockResolvedValue([]); + H.configValues = {}; + H.configListener = null; H.repos = [{ path: '/repo', name: 'repo', type: 'root' }]; (MainPanel as unknown as { currentPanel: unknown }).currentPanel = undefined; MainPanel.createOrShow(extUri, '/repo'); @@ -437,3 +452,43 @@ describe('MainPanel orchestration logic', () => { } }); }); + +describe('MainPanel showStashes setting', () => { + const lastLogArgs = () => H.git.log.mock.calls.at(-1)![0] as { includeStashes?: unknown }; + + it('getLog keeps stashes by default', async () => { + await dispatch({ type: 'getLog', payload: {} }); + expect(lastLogArgs().includeStashes).toBe(true); + }); + + it('getLog omits stashes when gitGraphPlus.showStashes is false', async () => { + H.configValues.showStashes = false; + await dispatch({ type: 'getLog', payload: {} }); + expect(lastLogArgs().includeStashes).toBe(false); + }); + + it('refreshAll omits stashes when gitGraphPlus.showStashes is false', async () => { + H.configValues.showStashes = false; + await (MainPanel.currentPanel as unknown as { refreshAll(): Promise }).refreshAll(); + expect(lastLogArgs().includeStashes).toBe(false); + }); + + it('search handlers follow the setting', async () => { + H.configValues.showStashes = false; + await dispatch({ type: 'searchCommits', payload: { query: 'x' } }); + expect(H.git.searchCommits).toHaveBeenCalledWith('x', expect.objectContaining({ includeStashes: false })); + + await dispatch({ type: 'searchByFile', payload: { file: 'a.ts' } }); + const call = H.git.searchByFile.mock.calls.at(-1)!; + expect(call[0]).toBe('a.ts'); + expect(call[2]).toEqual({ includeStashes: false }); + }); + + it('refreshes the graph when gitGraphPlus.showStashes changes', async () => { + expect(H.git.log).not.toHaveBeenCalled(); + H.configValues.showStashes = false; + H.configListener!({ affectsConfiguration: (k: string) => k === 'gitGraphPlus.showStashes' }); + await vi.waitFor(() => expect(H.git.log).toHaveBeenCalled()); + expect(lastLogArgs().includeStashes).toBe(false); + }); +}); diff --git a/src/utils/__tests__/config.test.ts b/src/utils/__tests__/config.test.ts index 63667d34..16150b2b 100644 --- a/src/utils/__tests__/config.test.ts +++ b/src/utils/__tests__/config.test.ts @@ -13,7 +13,7 @@ vi.mock('vscode', () => ({ }, })); -import { readTimeoutMs, readInitialCommitCount, readLoadMoreCommitCount } from '../config'; +import { readTimeoutMs, readInitialCommitCount, readLoadMoreCommitCount, readShowStashes } from '../config'; describe('readTimeoutMs', () => { // Back-compat alias so the existing timeout cases below read naturally. @@ -101,3 +101,23 @@ describe('readLoadMoreCommitCount', () => { expect(readLoadMoreCommitCount()).toBe(50); }); }); + +describe('readShowStashes', () => { + beforeEach(() => { h.values = {}; }); + + it('defaults to true when unset (stashes visible, current behaviour)', () => { + expect(readShowStashes()).toBe(true); + }); + + it('returns the boolean value as-is', () => { + h.values.showStashes = false; + expect(readShowStashes()).toBe(false); + h.values.showStashes = true; + expect(readShowStashes()).toBe(true); + }); + + it('falls back to true for a non-boolean value', () => { + h.values.showStashes = 'no'; + expect(readShowStashes()).toBe(true); + }); +}); diff --git a/src/utils/config.ts b/src/utils/config.ts index fa8fabed..481ac73e 100644 --- a/src/utils/config.ts +++ b/src/utils/config.ts @@ -47,3 +47,13 @@ export function readInteractiveRebaseMode(): InteractiveRebaseMode { vscode.workspace.getConfiguration('gitGraphPlus').get('interactiveRebase.mode', 'ui'), ); } + +/** + * Reads `gitGraphPlus.showStashes` — whether stashes are rendered in the + * commit graph and included in commit search results. Defaults to true + * (stashes visible) so existing behaviour is unchanged. + */ +export function readShowStashes(): boolean { + const raw = vscode.workspace.getConfiguration('gitGraphPlus').get('showStashes', true); + return typeof raw === 'boolean' ? raw : true; +}