From 239320a30f128ca9133eabcdc8b5cbd69acf3940 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Tue, 8 Feb 2022 03:02:38 -0500 Subject: [PATCH] Adds commit search support for GitHub Hides unsupported search options --- package.json | 8 +- src/commands/git/search.ts | 37 ++++--- src/env/node/git/localGitProvider.ts | 10 +- src/git/gitProvider.ts | 6 +- src/git/gitProviderService.ts | 2 +- src/premium/github/github.ts | 84 +++++++++++++- src/premium/github/githubGitProvider.ts | 189 +++++++++++++++++++++++++++----- 7 files changed, 284 insertions(+), 52 deletions(-) diff --git a/package.json b/package.json index 0d24025..0bc4173 100644 --- a/package.json +++ b/package.json @@ -10013,7 +10013,13 @@ }, { "view": "gitlens.views.searchAndCompare", - "contents": "Search for commits by [message](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22message%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D), [author](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22author%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D), [SHA](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22commit%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D), [file](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22file%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D), or [changes](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22change%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D)\n\n[Search Commits...](command:gitlens.views.searchAndCompare.searchCommits)" + "contents": "Search for commits by [message](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22message%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D), [author](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22author%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D), [SHA](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22commit%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D), [file](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22file%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D), or [changes](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22change%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D)\n\n[Search Commits...](command:gitlens.views.searchAndCompare.searchCommits)", + "when": "!gitlens:hasVirtualFolders" + }, + { + "view": "gitlens.views.searchAndCompare", + "contents": "Search for commits by [message](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22message%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D), [author](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22author%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D), or [SHA](command:gitlens.views.searchAndCompare.searchCommits?%7B%22search%22%3A%7B%22pattern%22%3A%22commit%3A%22%7D%2C%22prefillOnly%22%3Atrue%7D)\n\n[Search Commits...](command:gitlens.views.searchAndCompare.searchCommits)", + "when": "gitlens:hasVirtualFolders" }, { "view": "gitlens.views.searchAndCompare", diff --git a/src/commands/git/search.ts b/src/commands/git/search.ts index 99d382e..25ab36a 100644 --- a/src/commands/git/search.ts +++ b/src/commands/git/search.ts @@ -1,5 +1,6 @@ -import { GlyphChars } from '../../constants'; +import { ContextKeys, GlyphChars } from '../../constants'; import { Container } from '../../container'; +import { getContext } from '../../context'; import { GitCommit, GitLog, Repository } from '../../git/models'; import { searchOperators, SearchOperators, SearchPattern } from '../../git/search'; import { ActionQuickPickItem, QuickPickItemOfT } from '../../quickpicks/items/common'; @@ -25,6 +26,7 @@ interface Context { repos: Repository[]; associatedView: ViewsWithRepositoryFolders; commit: GitCommit | undefined; + hasVirtualFolders: boolean; resultsKey: string | undefined; resultsPromise: Promise | undefined; title: string; @@ -96,6 +98,7 @@ export class SearchGitCommand extends QuickCommand { repos: this.container.git.openRepositories, associatedView: this.container.searchAndCompareView, commit: undefined, + hasVirtualFolders: getContext(ContextKeys.HasVirtualFolders, false), resultsKey: undefined, resultsPromise: undefined, title: this.title, @@ -263,29 +266,33 @@ export class SearchGitCommand extends QuickCommand { { label: searchOperatorToTitleMap.get('')!, description: `pattern or message: pattern or =: pattern ${GlyphChars.Dash} use quotes to search for phrases`, - item: 'message:', + item: 'message:' as const, }, { label: searchOperatorToTitleMap.get('author:')!, description: 'author: pattern or @: pattern', - item: 'author:', + item: 'author:' as const, }, { label: searchOperatorToTitleMap.get('commit:')!, description: 'commit: sha or #: sha', - item: 'commit:', + item: 'commit:' as const, }, - { - label: searchOperatorToTitleMap.get('file:')!, - description: 'file: glob or ?: glob', - item: 'file:', - }, - { - label: searchOperatorToTitleMap.get('change:')!, - description: 'change: pattern or ~: pattern', - item: 'change:', - }, - ]; + context.hasVirtualFolders + ? undefined + : { + label: searchOperatorToTitleMap.get('file:')!, + description: 'file: glob or ?: glob', + item: 'file:' as const, + }, + context.hasVirtualFolders + ? undefined + : { + label: searchOperatorToTitleMap.get('change:')!, + description: 'change: pattern or ~: pattern', + item: 'change:' as const, + }, + ].filter((i?: T): i is T => i != null); const matchCaseButton = new QuickCommandButtons.MatchCaseToggle(state.matchCase); const matchAllButton = new QuickCommandButtons.MatchAllToggle(state.matchAll); diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 88816a7..a54b0f8 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -2095,7 +2095,7 @@ export class LocalGitProvider implements GitProvider, Disposable { async getLogForSearch( repoPath: string, search: SearchPattern, - options?: { limit?: number; ordering?: string | null; skip?: number }, + options?: { limit?: number; ordering?: 'date' | 'author-date' | 'topo' | null; skip?: number }, ): Promise { search = { matchAll: false, matchCase: false, matchRegex: true, ...search }; @@ -2216,7 +2216,7 @@ export class LocalGitProvider implements GitProvider, Disposable { private getLogForSearchMoreFn( log: GitLog, search: SearchPattern, - options?: { limit?: number; ordering?: string | null }, + options?: { limit?: number; ordering?: 'date' | 'author-date' | 'topo' | null }, ): (limit: number | undefined) => Promise { return async (limit: number | undefined) => { limit = limit ?? this.container.config.advanced.maxSearchItems ?? 0; @@ -2226,10 +2226,8 @@ export class LocalGitProvider implements GitProvider, Disposable { limit: limit, skip: log.count, }); - if (moreLog == null) { - // If we can't find any more, assume we have everything - return { ...log, hasMore: false }; - } + // If we can't find any more, assume we have everything + if (moreLog == null) return { ...log, hasMore: false }; const commits = new Map([...log.commits, ...moreLog.commits]); diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index f4defdc..e57474d 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -283,7 +283,11 @@ export interface GitProvider extends Disposable { getLogForSearch( repoPath: string, search: SearchPattern, - options?: { limit?: number | undefined; ordering?: string | null | undefined; skip?: number | undefined }, + options?: { + limit?: number | undefined; + ordering?: 'date' | 'author-date' | 'topo' | null | undefined; + skip?: number | undefined; + }, ): Promise; getLogForFile( repoPath: string, diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index c51cf15..fed91fb 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -1224,7 +1224,7 @@ export class GitProviderService implements Disposable { async getLogForSearch( repoPath: string | Uri, search: SearchPattern, - options?: { limit?: number; ordering?: string | null; skip?: number }, + options?: { limit?: number; ordering?: 'date' | 'author-date' | 'topo' | null; skip?: number }, ): Promise { const { provider, path } = this.getProvider(repoPath); return provider.getLogForSearch(path, search, options); diff --git a/src/premium/github/github.ts b/src/premium/github/github.ts index 6e206bc..55c8ac5 100644 --- a/src/premium/github/github.ts +++ b/src/premium/github/github.ts @@ -1528,6 +1528,84 @@ export class GitHubApi { } } + @debug({ args: { 0: '' } }) + async searchCommits( + token: string, + query: string, + options?: { + cursor?: string; + limit?: number; + order?: 'asc' | 'desc' | undefined; + sort?: 'author-date' | 'committer-date' | undefined; + }, + ): Promise | undefined> { + const cc = Logger.getCorrelationContext(); + + const limit = Math.min(100, options?.limit ?? 100); + + let page; + let pageSize; + let previousCount; + if (options?.cursor != null) { + [page, pageSize, previousCount] = options.cursor.split(' ', 3); + page = parseInt(page, 10); + // TODO@eamodio need to figure out how allow different page sizes if the limit changes + pageSize = parseInt(pageSize, 10); + previousCount = parseInt(previousCount, 10); + } else { + page = 1; + pageSize = limit; + previousCount = 0; + } + + try { + const rsp = await this.request(token, 'GET /search/commits', { + q: query, + sort: options?.sort, + order: options?.order, + per_page: pageSize, + page: page, + }); + + const data = rsp?.data; + if (data == null) return undefined; + + const commits = data.items.map(result => ({ + oid: result.sha, + parents: { nodes: result.parents.map(p => ({ oid: p.sha! })) }, + message: result.commit.message, + author: { + avatarUrl: result.author?.avatar_url ?? undefined, + date: result.commit.author?.date ?? result.commit.author?.date ?? new Date().toString(), + email: result.author?.email ?? result.commit.author?.email ?? undefined, + name: result.author?.name ?? result.commit.author?.name ?? '', + }, + committer: { + date: result.commit.committer?.date ?? result.committer?.date ?? new Date().toString(), + email: result.committer?.email ?? result.commit.committer?.email ?? undefined, + name: result.committer?.name ?? result.commit.committer?.name ?? '', + }, + })); + + const count = previousCount + data.items.length; + const hasMore = data.incomplete_results || data.total_count > count; + + return { + pageInfo: { + startCursor: `${page} ${pageSize} ${previousCount}`, + endCursor: hasMore ? `${page + 1} ${pageSize} ${count}` : undefined, + hasPreviousPage: data.total_count > 0 && page > 1, + hasNextPage: hasMore, + }, + totalCount: data.total_count, + values: commits, + }; + } catch (ex) { + debugger; + return this.handleRequestError(ex, cc, undefined); + } + } + private _octokits = new Map(); private octokit(token: string, options?: ConstructorParameters[0]): Octokit { let octokit = this._octokits.get(token); @@ -1665,9 +1743,9 @@ export interface GitHubCommit { oid: string; parents: { nodes: { oid: string }[] }; message: string; - additions: number | undefined; - changedFiles: number | undefined; - deletions: number | undefined; + additions?: number | undefined; + changedFiles?: number | undefined; + deletions?: number | undefined; author: { avatarUrl: string | undefined; date: string; email: string | undefined; name: string }; committer: { date: string; email: string | undefined; name: string }; diff --git a/src/premium/github/githubGitProvider.ts b/src/premium/github/githubGitProvider.ts index 30dda6c..524a4ed 100644 --- a/src/premium/github/githubGitProvider.ts +++ b/src/premium/github/githubGitProvider.ts @@ -1166,7 +1166,6 @@ export class GitHubGitProvider implements GitProvider, Disposable { since: options?.since ? new Date(options.since) : undefined, }); - const authors = new Map(); const commits = new Map(); const { viewer = session.account.label } = result; @@ -1175,15 +1174,6 @@ export class GitHubGitProvider implements GitProvider, Disposable { const committerName = viewer != null && commit.committer.name === viewer ? 'You' : commit.committer.name; - let author = authors.get(authorName); - if (author == null) { - author = { - name: authorName, - lineCount: 0, - }; - authors.set(authorName, author); - } - let c = commits.get(commit.oid); if (c == null) { c = new GitCommit( @@ -1330,7 +1320,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { limit: moreUntil == null ? (log.limit ?? 0) + moreLimit : undefined, hasMore: moreUntil == null ? moreLog.hasMore : true, cursor: moreLog.cursor, - query: moreLog.query, + query: log.query, }; mergedLog.more = this.getLogMoreFn(mergedLog, options); @@ -1342,13 +1332,16 @@ export class GitHubGitProvider implements GitProvider, Disposable { async getLogForSearch( repoPath: string, search: SearchPattern, - _options?: { limit?: number; ordering?: string | null; skip?: number }, + options?: { cursor?: string; limit?: number; ordering?: 'date' | 'author-date' | 'topo' | null; skip?: number }, ): Promise { if (repoPath == null) return undefined; + const cc = Logger.getCorrelationContext(); + const operations = SearchPattern.parseSearchOperations(search.pattern); - const values = operations.get('commit:'); + let op; + let values = operations.get('commit:'); if (values != null) { const commit = await this.getCommit(repoPath, values[0]); if (commit == null) return undefined; @@ -1364,10 +1357,166 @@ export class GitHubGitProvider implements GitProvider, Disposable { }; } - // TODO@eamodio try implementing with the commit search api + const query = []; + + for ([op, values] of operations.entries()) { + switch (op) { + case 'message:': + query.push(...values.map(m => m.replace(/ /g, '+'))); + break; + + case 'author:': + query.push( + ...values.map(a => { + a = a.replace(/ /g, '+'); + if (a.startsWith('@')) return `author:${a.slice(1)}`; + if (a.startsWith('"@')) return `author:"${a.slice(2)}`; + if (a.includes('@')) return `author-email:${a}`; + return `author-name:${a}`; + }), + ); + break; + + // case 'change:': + // case 'file:': + // break; + } + } + + if (query.length === 0) return undefined; + + const limit = this.getPagingLimit(options?.limit); + + try { + const { metadata, github, session } = await this.ensureRepositoryContext(repoPath); + + const result = await github.searchCommits( + session?.accessToken, + `repo:${metadata.repo.owner}/${metadata.repo.name}+${query.join('+').trim()}`, + { + cursor: options?.cursor, + limit: limit, + sort: + options?.ordering === 'date' + ? 'committer-date' + : options?.ordering === 'author-date' + ? 'author-date' + : undefined, + }, + ); + if (result == null) return undefined; + + const commits = new Map(); + + const viewer = session.account.label; + for (const commit of result.values) { + const authorName = viewer != null && commit.author.name === viewer ? 'You' : commit.author.name; + const committerName = + viewer != null && commit.committer.name === viewer ? 'You' : commit.committer.name; + + let c = commits.get(commit.oid); + if (c == null) { + c = new GitCommit( + this.container, + repoPath, + commit.oid, + new GitCommitIdentity( + authorName, + commit.author.email, + new Date(commit.author.date), + commit.author.avatarUrl, + ), + new GitCommitIdentity(committerName, commit.committer.email, new Date(commit.committer.date)), + commit.message.split('\n', 1)[0], + commit.parents.nodes.map(p => p.oid), + commit.message, + commit.files?.map( + f => + new GitFileChange( + repoPath, + f.filename ?? '', + fromCommitFileStatus(f.status) ?? GitFileIndexStatus.Modified, + f.previous_filename, + undefined, + { + additions: f.additions ?? 0, + deletions: f.deletions ?? 0, + changes: f.changes ?? 0, + }, + ), + ), + { + changedFiles: commit.changedFiles ?? 0, + additions: commit.additions ?? 0, + deletions: commit.deletions ?? 0, + }, + [], + ); + commits.set(commit.oid, c); + } + } + + const log: GitLog = { + repoPath: repoPath, + commits: commits, + sha: undefined, + range: undefined, + count: commits.size, + limit: limit, + hasMore: result.pageInfo?.hasNextPage ?? false, + cursor: result.pageInfo?.endCursor ?? undefined, + query: (limit: number | undefined) => this.getLog(repoPath, { ...options, limit: limit }), + }; + + if (log.hasMore) { + log.more = this.getLogForSearchMoreFn(log, search, options); + } + + return log; + } catch (ex) { + Logger.error(ex, cc); + debugger; + return undefined; + } + return undefined; } + private getLogForSearchMoreFn( + log: GitLog, + search: SearchPattern, + options?: { limit?: number; ordering?: 'date' | 'author-date' | 'topo' | null; skip?: number }, + ): (limit: number | undefined) => Promise { + return async (limit: number | undefined) => { + limit = this.getPagingLimit(limit); + + const moreLog = await this.getLogForSearch(log.repoPath, search, { + ...options, + limit: limit, + cursor: log.cursor, + }); + // If we can't find any more, assume we have everything + if (moreLog == null) return { ...log, hasMore: false }; + + const commits = new Map([...log.commits, ...moreLog.commits]); + + const mergedLog: GitLog = { + repoPath: log.repoPath, + commits: commits, + sha: log.sha, + range: undefined, + count: commits.size, + limit: (log.limit ?? 0) + limit, + hasMore: moreLog.hasMore, + cursor: moreLog.cursor, + query: log.query, + }; + mergedLog.more = this.getLogForSearchMoreFn(mergedLog, search, options); + + return mergedLog; + }; + } + @log() async getLogForFile( repoPath: string | undefined, @@ -1571,7 +1720,6 @@ export class GitHubGitProvider implements GitProvider, Disposable { since: options?.since ? new Date(options.since) : undefined, }); - const authors = new Map(); const commits = new Map(); const { viewer = session.account.label } = result; @@ -1580,15 +1728,6 @@ export class GitHubGitProvider implements GitProvider, Disposable { const committerName = viewer != null && commit.committer.name === viewer ? 'You' : commit.committer.name; - let author = authors.get(authorName); - if (author == null) { - author = { - name: authorName, - lineCount: 0, - }; - authors.set(authorName, author); - } - let c = commits.get(commit.oid); if (c == null) { const files = commit.files?.map( @@ -1724,7 +1863,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { limit: moreUntil == null ? (log.limit ?? 0) + moreLimit : undefined, hasMore: moreUntil == null ? moreLog.hasMore : true, cursor: moreLog.cursor, - query: moreLog.query, + query: log.query, }; // if (options.renames) {