From 084f580fc4791465837abc0d85f765ae78bdcd81 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Fri, 26 Aug 2022 03:23:20 -0400 Subject: [PATCH] Adds opening/showing specific commit in the Graph - Adds Show Commit in Graph to the Commit Details view Attempts to fix Graph data paging using `--boundary` --- src/constants.ts | 1 + src/env/node/git/git.ts | 14 ++ src/env/node/git/localGitProvider.ts | 168 +++++++++++++++++++-- src/git/models/graph.ts | 1 + src/git/parsers/logParser.ts | 3 +- src/plus/github/githubGitProvider.ts | 5 +- src/plus/webviews/graph/graphWebview.ts | 27 +++- src/plus/webviews/graph/protocol.ts | 1 + src/webviews/apps/commitDetails/commitDetails.html | 17 ++- src/webviews/apps/commitDetails/commitDetails.ts | 24 +-- src/webviews/apps/plus/graph/GraphWrapper.tsx | 4 + src/webviews/apps/shared/components/codicon.ts | 4 + src/webviews/apps/shared/theme.ts | 2 +- .../commitDetails/commitDetailsWebviewView.ts | 8 + src/webviews/commitDetails/protocol.ts | 2 +- 15 files changed, 237 insertions(+), 44 deletions(-) diff --git a/src/constants.ts b/src/constants.ts index aeab79d..16a8bf5f 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -163,6 +163,7 @@ export const enum Commands { SearchCommitsInView = 'gitlens.views.searchAndCompare.searchCommits', SetViewsLayout = 'gitlens.setViewsLayout', ShowBranchesView = 'gitlens.showBranchesView', + ShowCommitInGraph = 'gitlens.showCommitInGraph', ShowCommitInView = 'gitlens.showCommitInView', ShowCommitsInView = 'gitlens.showCommitsInView', ShowCommitsView = 'gitlens.showCommitsView', diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index 93ce13a..76e35d3 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -769,6 +769,20 @@ export class Git { ); } + log2(repoPath: string, ref: string | undefined, ...args: unknown[]) { + const params = ['log', ...args]; + + if (ref && !GitRevision.isUncommittedStaged(ref)) { + params.push(ref); + } + + return this.git( + { cwd: repoPath, configs: ['-c', 'diff.renameLimit=0', '-c', 'log.showSignature=false'] }, + ...params, + '--', + ); + } + log__file( repoPath: string, fileName: string, diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 11516ca..f082bea 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -1609,8 +1609,75 @@ export class LocalGitProvider implements GitProvider, Disposable { ref?: string; }, ): Promise { + const scope = getLogScope(); + + let getLogForRefFn; + if (options?.ref != null) { + async function getLogForRef(this: LocalGitProvider): Promise { + let log; + + const parser = GitLogParser.create<{ sha: string; date: string }>({ sha: '%H', date: '%ct' }); + const data = await this.git.log(repoPath, options?.ref, { argsOrFormat: parser.arguments, limit: 0 }); + + let commit = first(parser.parse(data)); + if (commit != null) { + log = await this.getLog(repoPath, { + all: options!.mode !== 'single', + ordering: 'date', + limit: 0, + extraArgs: [`--since="${Number(commit.date)}"`, '--boundary'], + }); + + let found = log?.commits.has(commit.sha) ?? false; + if (!found) { + Logger.debug(scope, `Could not find commit ${options!.ref}`); + + debugger; + } + + if (log?.more != null) { + const defaultItemLimit = configuration.get('graph.defaultItemLimit'); + if (!found || log.commits.size < defaultItemLimit) { + Logger.debug(scope, 'Loading next page...'); + + log = await log.more( + (log.commits.size < defaultItemLimit + ? defaultItemLimit + : configuration.get('graph.pageItemLimit')) ?? options?.limit, + ); + // We need to clear the "pagedCommits", since we want to return the entire set + if (log != null) { + (log as Mutable).pagedCommits = undefined; + } + + found = log?.commits.has(commit.sha) ?? false; + if (!found) { + Logger.debug(scope, `Still could not find commit ${options!.ref}`); + commit = undefined; + + debugger; + } + } + } + + if (!found) { + commit = undefined; + } + + options!.ref = commit?.sha; + } + return ( + log ?? + this.getLog(repoPath, { all: options?.mode !== 'single', ordering: 'date', limit: options?.limit }) + ); + } + + getLogForRefFn = getLogForRef; + } + const [logResult, stashResult, remotesResult] = await Promise.allSettled([ - this.getLog(repoPath, { all: true, ordering: 'date', limit: options?.limit }), + getLogForRefFn?.call(this) ?? + this.getLog(repoPath, { all: options?.mode !== 'single', ordering: 'date', limit: options?.limit }), this.getStash(repoPath), this.getRemotes(repoPath), ]); @@ -1632,9 +1699,10 @@ export class LocalGitProvider implements GitProvider, Disposable { stash: GitStash | undefined, remotes: GitRemote[] | undefined, options?: { - ref?: string; - mode?: 'single' | 'local' | 'all'; branch?: string; + limit?: number; + mode?: 'single' | 'local' | 'all'; + ref?: string; }, ): Promise { if (log == null) { @@ -1763,6 +1831,7 @@ export class LocalGitProvider implements GitProvider, Disposable { more: log.hasMore, }, rows: rows, + sha: options?.ref, more: async (limit: number | { until: string } | undefined): Promise => { const moreLog = await log.more?.(limit); @@ -2256,23 +2325,79 @@ export class LocalGitProvider implements GitProvider, Disposable { ref?: string; since?: number | string; until?: number | string; + extraArgs?: string[]; }, ): Promise { const scope = getLogScope(); - const limit = options?.limit ?? configuration.get('advanced.maxListItems') ?? 0; - try { + const limit = options?.limit ?? configuration.get('advanced.maxListItems') ?? 0; + const merges = options?.merges == null ? true : options.merges; + const ordering = options?.ordering ?? configuration.get('advanced.commitOrdering'); + const similarityThreshold = configuration.get('advanced.similarityThreshold'); + + const args = [ + `--format=${options?.all ? GitLogParser.allFormat : GitLogParser.defaultFormat}`, + '--name-status', + '--full-history', + `-M${similarityThreshold == null ? '' : `${similarityThreshold}%`}`, + '-m', + ]; + if (options?.all) { + args.push('--all'); + } + if (!merges) { + args.push('--first-parent'); + } + if (ordering) { + args.push(`--${ordering}-order`); + } + if (options?.authors?.length) { + args.push( + '--use-mailmap', + '--author', + ...options.authors.map(a => `--author=^${a.name} <${a.email}>$`), + ); + } + + let hasMoreOverride; + + if (options?.since) { + hasMoreOverride = true; + args.push(`--since="${options.since}"`); + } + if (options?.until) { + hasMoreOverride = true; + args.push(`--until="${options.until}"`); + } + if (options?.extraArgs?.length) { + if ( + options.extraArgs.some( + arg => arg.startsWith('-n') || arg.startsWith('--until=') || arg.startsWith('--since='), + ) + ) { + hasMoreOverride = true; + } + args.push(...options.extraArgs); + } + + if (limit) { + hasMoreOverride = undefined; + args.push(`-n${limit + 1}`); + } + + const data = await this.git.log2(repoPath, options?.ref, ...args); + // const parser = GitLogParser.defaultParser; - const data = await this.git.log(repoPath, options?.ref, { - ...options, - // args: parser.arguments, - limit: limit, - merges: options?.merges == null ? true : options.merges, - ordering: options?.ordering ?? configuration.get('advanced.commitOrdering'), - similarityThreshold: configuration.get('advanced.similarityThreshold'), - }); + // const data = await this.git.log2(repoPath, options?.ref, { + // ...options, + // // args: parser.arguments, + // limit: limit, + // merges: options?.merges == null ? true : options.merges, + // ordering: options?.ordering ?? configuration.get('advanced.commitOrdering'), + // similarityThreshold: configuration.get('advanced.similarityThreshold'), + // }); // const commits = []; // const entries = parser.parse(data); @@ -2311,12 +2436,18 @@ export class LocalGitProvider implements GitProvider, Disposable { limit, false, undefined, + hasMoreOverride, ); if (log != null) { log.query = (limit: number | undefined) => this.getLog(repoPath, { ...options, limit: limit }); if (log.hasMore) { - log.more = this.getLogMoreFn(log, options); + let opts; + if (options != null) { + let extraArgs; + ({ extraArgs, ...opts } = options); + } + log.more = this.getLogMoreFn(log, opts); } } @@ -2414,12 +2545,19 @@ export class LocalGitProvider implements GitProvider, Disposable { ...options, limit: moreUntil == null ? moreLimit : 0, ...(timestamp - ? { until: timestamp } + ? { + until: timestamp, + extraArgs: ['--boundary'], + } : { ref: moreUntil == null ? `${ref}^` : `${moreUntil}^..${ref}^` }), }); // If we can't find any more, assume we have everything if (moreLog == null) return { ...log, hasMore: false, more: undefined }; + if (timestamp != null && ref != null && !moreLog.commits.has(ref)) { + debugger; + } + const commits = new Map([...log.commits, ...moreLog.commits]); const mergedLog: GitLog = { diff --git a/src/git/models/graph.ts b/src/git/models/graph.ts index 8c70362..d82d47e 100644 --- a/src/git/models/graph.ts +++ b/src/git/models/graph.ts @@ -22,6 +22,7 @@ export interface GitGraphRow extends GraphRow { export interface GitGraph { readonly repoPath: string; readonly rows: GitGraphRow[]; + readonly sha?: string; readonly paging?: { readonly limit: number | undefined; diff --git a/src/git/parsers/logParser.ts b/src/git/parsers/logParser.ts index 10a17bc..1e1759b 100644 --- a/src/git/parsers/logParser.ts +++ b/src/git/parsers/logParser.ts @@ -295,6 +295,7 @@ export class GitLogParser { limit: number | undefined, reverse: boolean, range: Range | undefined, + hasMoreOverride?: boolean, ): GitLog | undefined { if (!data) return undefined; @@ -595,7 +596,7 @@ export class GitLogParser { count: i, limit: limit, range: range, - hasMore: Boolean(truncationCount && i > truncationCount && truncationCount !== 1), + hasMore: hasMoreOverride ?? Boolean(truncationCount && i > truncationCount && truncationCount !== 1), }; return log; } diff --git a/src/plus/github/githubGitProvider.ts b/src/plus/github/githubGitProvider.ts index e13aa7b..58b3852 100644 --- a/src/plus/github/githubGitProvider.ts +++ b/src/plus/github/githubGitProvider.ts @@ -1076,9 +1076,10 @@ export class GitHubGitProvider implements GitProvider, Disposable { remote: GitRemote | undefined, tags: GitTag[] | undefined, options?: { - ref?: string; - mode?: 'single' | 'local' | 'all'; branch?: string; + limit?: number; + mode?: 'single' | 'local' | 'all'; + ref?: string; }, ): Promise { if (log == null) { diff --git a/src/plus/webviews/graph/graphWebview.ts b/src/plus/webviews/graph/graphWebview.ts index 1ba681e..b8286e2 100644 --- a/src/plus/webviews/graph/graphWebview.ts +++ b/src/plus/webviews/graph/graphWebview.ts @@ -38,6 +38,10 @@ import { UpdateSelectionCommandType, } from './protocol'; +export interface ShowCommitInGraphCommandArgs { + sha: string; +} + export interface GraphSelectionChangeEvent { readonly selection: GitCommit[]; } @@ -74,6 +78,7 @@ export class GraphWebview extends WebviewBase { private _etagSubscription?: number; private _etagRepository?: number; + private _selectedSha?: string; private _repositoryEventsDisposable: Disposable | undefined; private _repositoryGraph?: GitGraph; @@ -101,6 +106,15 @@ export class GraphWebview extends WebviewBase { }, }, this.container.subscription.onDidChange(this.onSubscriptionChanged, this), + registerCommand(Commands.ShowCommitInGraph, (args: ShowCommitInGraphCommandArgs) => { + this._selectedSha = args.sha; + if (this._panel == null) { + void this.show(); + } else { + // TODO@eamodio we should be smarter here an look for the commit in the saved data before refreshing and only send the selectedSha + this.updateState(); + } + }), ); this.onConfigurationChanged(); @@ -123,7 +137,6 @@ export class GraphWebview extends WebviewBase { } if (this.repository != null) { - this.resetRepositoryState(); this.updateState(); } } @@ -301,11 +314,12 @@ export class GraphWebview extends WebviewBase { } private async onSelectionChanged(selection: string[]) { - const ref = selection[0]; + const sha = selection[0]; + this._selectedSha = sha; let commits: GitCommit[] | undefined; - if (ref != null) { - const commit = await this.repository?.getCommit(ref); + if (sha != null) { + const commit = await this.repository?.getCommit(sha); if (commit != null) { commits = [commit]; } @@ -421,14 +435,16 @@ export class GraphWebview extends WebviewBase { const data = await this.container.git.getCommitsForGraph( this.repository.path, this._panel!.webview.asWebviewUri, - { limit: limit }, + { limit: limit, ref: this._selectedSha ?? 'HEAD' }, ); this._repositoryGraph = data; + this._selectedSha = data.sha; return { previewBanner: this.previewBanner, repositories: formatRepositories(this.container.git.openRepositories), selectedRepository: this.repository.path, + selectedSha: this._selectedSha, selectedVisibility: access.visibility, subscription: access.subscription.current, allowed: access.allowed, @@ -445,6 +461,7 @@ export class GraphWebview extends WebviewBase { private resetRepositoryState() { this._repositoryGraph = undefined; + this._selectedSha = undefined; } } diff --git a/src/plus/webviews/graph/protocol.ts b/src/plus/webviews/graph/protocol.ts index 9bfc76c..0ff2936 100644 --- a/src/plus/webviews/graph/protocol.ts +++ b/src/plus/webviews/graph/protocol.ts @@ -7,6 +7,7 @@ import { IpcCommandType, IpcNotificationType } from '../../../webviews/protocol' export interface State { repositories?: GraphRepository[]; selectedRepository?: string; + selectedSha?: string; selectedVisibility?: RepositoryVisibility; subscription?: Subscription; allowed?: boolean; diff --git a/src/webviews/apps/commitDetails/commitDetails.html b/src/webviews/apps/commitDetails/commitDetails.html index ef4ce3f..417e69a 100644 --- a/src/webviews/apps/commitDetails/commitDetails.html +++ b/src/webviews/apps/commitDetails/commitDetails.html @@ -43,7 +43,7 @@ + > { DOM.on('file-change-item', 'file-more-actions', e => this.onFileMoreActions(e.detail), ), - DOM.on('[data-action="commit-actions-sha"]', 'click', e => this.onCommitShaActions(e)), - DOM.on('[data-action="commit-actions-more"]', 'click', e => this.onCommitMoreActions(e)), + DOM.on('[data-action="commit-actions"]', 'click', e => this.onCommitActions(e)), DOM.on('[data-action="pick-commit"]', 'click', e => this.onPickCommit(e)), DOM.on('[data-action="search-commit"]', 'click', e => this.onSearchCommit(e)), DOM.on('[data-action="autolink-settings"]', 'click', e => this.onAutolinkSettings(e)), @@ -82,7 +81,7 @@ export class CommitDetailsApp extends App> { $next?.focus(); } }), - DOM.on('[data-action="commit-actions-pin"]', 'click', e => this.onTogglePin(e)), + DOM.on('[data-action="pin"]', 'click', e => this.onTogglePin(e)), DOM.on( '[data-region="rich-pane"]', 'expanded-change', @@ -173,24 +172,17 @@ export class CommitDetailsApp extends App> { this.sendCommand(FileActionsCommandType, e); } - private onCommitMoreActions(e: MouseEvent) { + private onCommitActions(e: MouseEvent) { e.preventDefault(); if (this.state.selected === undefined) { e.stopPropagation(); return; } - this.sendCommand(CommitActionsCommandType, { action: 'more' }); - } - - private onCommitShaActions(e: MouseEvent) { - e.preventDefault(); - if (this.state.selected === undefined) { - e.stopPropagation(); - return; - } + const action = (e.target as HTMLElement)?.getAttribute('data-action-type'); + if (action == null) return; - this.sendCommand(CommitActionsCommandType, { action: 'sha', alt: e.altKey }); + this.sendCommand(CommitActionsCommandType, { action: action as CommitActionsParams['action'], alt: e.altKey }); } renderCommit(state: Serialized): state is CommitState { @@ -228,7 +220,7 @@ export class CommitDetailsApp extends App> { } renderPin(state: CommitState) { - const $el = document.querySelector('[data-action="commit-actions-pin"]'); + const $el = document.querySelector('[data-action="pin"]'); if ($el == null) { return; } diff --git a/src/webviews/apps/plus/graph/GraphWrapper.tsx b/src/webviews/apps/plus/graph/GraphWrapper.tsx index d6211d5..fb82253 100644 --- a/src/webviews/apps/plus/graph/GraphWrapper.tsx +++ b/src/webviews/apps/plus/graph/GraphWrapper.tsx @@ -117,6 +117,7 @@ export function GraphWrapper({ repositories = [], rows = [], selectedRepository, + selectedSha, subscription, allowed, config, @@ -135,6 +136,7 @@ export function GraphWrapper({ const [currentRepository, setCurrentRepository] = useState( reposList.find(item => item.path === selectedRepository), ); + const [currentSha, setSelectedSha] = useState(selectedSha); const [graphColSettings, setGraphColSettings] = useState(getGraphColSettingsModel(config)); const [pagingState, setPagingState] = useState(paging); const [isLoading, setIsLoading] = useState(false); @@ -179,6 +181,7 @@ export function GraphWrapper({ setGraphList(state.rows ?? []); setReposList(state.repositories ?? []); setCurrentRepository(reposList.find(item => item.path === state.selectedRepository)); + setSelectedSha(state.selectedSha); setGraphColSettings(getGraphColSettingsModel(state.config)); setPagingState(state.paging); setIsLoading(false); @@ -402,6 +405,7 @@ export function GraphWrapper({ getExternalIcon={getIconElementLibrary} graphRows={graphList} height={mainHeight} + isSelectedBySha={currentSha ? { [currentSha]: true } : undefined} hasMoreCommits={pagingState?.more} isLoadingRows={isLoading} nonce={nonce} diff --git a/src/webviews/apps/shared/components/codicon.ts b/src/webviews/apps/shared/components/codicon.ts index ccee086..c4739f5 100644 --- a/src/webviews/apps/shared/components/codicon.ts +++ b/src/webviews/apps/shared/components/codicon.ts @@ -1500,6 +1500,10 @@ export class CodeIcon extends LitElement { position: relative; left: 1px; } + :host([icon='graph']):before { + font-family: 'glicons'; + content: '\\f102'; + } `; @property() diff --git a/src/webviews/apps/shared/theme.ts b/src/webviews/apps/shared/theme.ts index b895d33..ddfce98 100644 --- a/src/webviews/apps/shared/theme.ts +++ b/src/webviews/apps/shared/theme.ts @@ -8,7 +8,7 @@ export function initializeAndWatchThemeColors(callback?: () => void) { const isLightTheme = body.classList.contains('vscode-light') || body.classList.contains('vscode-high-contrast-light'); - const isHighContrastTheme = body.classList.contains('vscode-high-contrast'); + // const isHighContrastTheme = body.classList.contains('vscode-high-contrast'); const bodyStyle = body.style; diff --git a/src/webviews/commitDetails/commitDetailsWebviewView.ts b/src/webviews/commitDetails/commitDetailsWebviewView.ts index 9acdcb7..336b359 100644 --- a/src/webviews/commitDetails/commitDetailsWebviewView.ts +++ b/src/webviews/commitDetails/commitDetailsWebviewView.ts @@ -9,6 +9,7 @@ import type { GitFileChange } from '../../git/models/file'; import { GitFile } from '../../git/models/file'; import type { IssueOrPullRequest } from '../../git/models/issue'; import type { PullRequest } from '../../git/models/pullRequest'; +import type { ShowCommitInGraphCommandArgs } from '../../plus/webviews/graph/graphWebview'; import { executeCommand } from '../../system/command'; import { debug } from '../../system/decorators/log'; import type { Deferrable } from '../../system/function'; @@ -180,6 +181,13 @@ export class CommitDetailsWebviewView extends WebviewViewBase { switch (params.action) { + case 'graph': + if (this._context.commit == null) return; + + void executeCommand(Commands.ShowCommitInGraph, { + sha: this._context.commit.sha, + }); + break; case 'more': this.showCommitActions(); break; diff --git a/src/webviews/commitDetails/protocol.ts b/src/webviews/commitDetails/protocol.ts index 58571f9..d5cad9a 100644 --- a/src/webviews/commitDetails/protocol.ts +++ b/src/webviews/commitDetails/protocol.ts @@ -45,7 +45,7 @@ export type ShowCommitDetailsViewCommandArgs = string[]; // COMMANDS export interface CommitActionsParams { - action: 'sha' | 'more'; + action: 'graph' | 'more' | 'sha'; alt?: boolean; } export const CommitActionsCommandType = new IpcCommandType('commit/actions');