From 3a6a1d0030e338ad9ac800a5b08a83956bd06434 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Wed, 12 Apr 2023 02:17:01 -0400 Subject: [PATCH] Adds plumbing for showing unsafe repos state --- src/env/node/git/git.ts | 16 ++++++++------ src/env/node/git/localGitProvider.ts | 13 ++++++++++- src/env/node/git/vslsGitProvider.ts | 8 ++++++- src/git/gitProvider.ts | 1 + src/git/gitProviderService.ts | 8 +++++++ src/webviews/apps/home/components/plus-banner.ts | 14 ++++++------ src/webviews/apps/home/home.ts | 21 ++++++++++-------- src/webviews/home/homeWebview.ts | 28 +++++++++++++----------- src/webviews/home/protocol.ts | 16 +++++++++----- 9 files changed, 81 insertions(+), 44 deletions(-) diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index 76a258d..7e7ba2f 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -1604,7 +1604,7 @@ export class Git { return { path: dotGitPath }; } - async rev_parse__show_toplevel(cwd: string): Promise { + async rev_parse__show_toplevel(cwd: string): Promise<[safe: true, repoPath: string] | [safe: false] | []> { let data; if (!workspace.isTrusted) { @@ -1618,7 +1618,7 @@ export class Git { ); if (data.trim() === '') { Logger.log(`Skipping (untrusted workspace); bare clone repository detected in '${cwd}'`); - return undefined; + return emptyArray as []; } } catch { // If this throw, we should be good to open the repo (e.g. HEAD doesn't exist) @@ -1629,7 +1629,9 @@ export class Git { data = await this.git({ cwd: cwd, errors: GitErrorHandling.Throw }, 'rev-parse', '--show-toplevel'); // Make sure to normalize: https://github.com/git-for-windows/git/issues/2478 // Keep trailing spaces which are part of the directory name - return data.length === 0 ? undefined : normalizePath(data.trimStart().replace(/[\r|\n]+$/, '')); + return data.length === 0 + ? (emptyArray as []) + : [true, normalizePath(data.trimStart().replace(/[\r|\n]+$/, ''))]; } catch (ex) { const unsafeMatch = /^fatal: detected dubious ownership in repository at '([^']+)'[\s\S]*git config --global --add safe\.directory '?([^'\n]+)'?$/m.exec( @@ -1639,7 +1641,7 @@ export class Git { Logger.log( `Skipping; unsafe repository detected in '${unsafeMatch[1]}'; run 'git config --global --add safe.directory ${unsafeMatch[2]}' to allow it`, ); - return undefined; + return [false]; } const inDotGit = /this operation must be run in a work tree/.test(ex.stderr); @@ -1659,7 +1661,7 @@ export class Git { ); data = data.trim(); if (data.length) { - return normalizePath((data === '.' ? cwd : data).trimStart().replace(/[\r|\n]+$/, '')); + return [true, normalizePath((data === '.' ? cwd : data).trimStart().replace(/[\r|\n]+$/, ''))]; } } } @@ -1670,7 +1672,7 @@ export class Git { if (!exists) { do { const parent = dirname(cwd); - if (parent === cwd || parent.length === 0) return undefined; + if (parent === cwd || parent.length === 0) return emptyArray as []; cwd = parent; exists = await fsExists(cwd); @@ -1679,7 +1681,7 @@ export class Git { return this.rev_parse__show_toplevel(cwd); } } - return undefined; + return emptyArray as []; } } diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 6b389d3..c399ad2 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -1066,6 +1066,7 @@ export class LocalGitProvider implements GitProvider, Disposable { private readonly toCanonicalMap = new Map(); private readonly fromCanonicalMap = new Map(); + protected readonly unsafePaths = new Set(); @gate() @debug() @@ -1084,7 +1085,13 @@ export class LocalGitProvider implements GitProvider, Disposable { uri = Uri.joinPath(uri, '..'); } - repoPath = await this.git.rev_parse__show_toplevel(uri.fsPath); + let safe; + [safe, repoPath] = await this.git.rev_parse__show_toplevel(uri.fsPath); + if (safe) { + this.unsafePaths.delete(uri.fsPath); + } else { + this.unsafePaths.add(uri.fsPath); + } if (!repoPath) return undefined; const repoUri = Uri.file(repoPath); @@ -4103,6 +4110,10 @@ export class LocalGitProvider implements GitProvider, Disposable { return this.git.merge_base__is_ancestor(repoPath, ref, '@{u}'); } + hasUnsafeRepositories(): boolean { + return this.unsafePaths.size !== 0; + } + isTrackable(uri: Uri): boolean { return this.supportedSchemes.has(uri.scheme); } diff --git a/src/env/node/git/vslsGitProvider.ts b/src/env/node/git/vslsGitProvider.ts index 22dc5d3..663a475 100644 --- a/src/env/node/git/vslsGitProvider.ts +++ b/src/env/node/git/vslsGitProvider.ts @@ -93,7 +93,13 @@ export class VslsGitProvider extends LocalGitProvider { uri = Uri.joinPath(uri, '..'); } - repoPath = await this.git.rev_parse__show_toplevel(uri.fsPath); + let safe; + [safe, repoPath] = await this.git.rev_parse__show_toplevel(uri.fsPath); + if (safe) { + this.unsafePaths.delete(uri.fsPath); + } else { + this.unsafePaths.add(uri.fsPath); + } if (!repoPath) return undefined; return repoPath ? Uri.parse(repoPath, true) : undefined; diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index 4204295..cbf523e 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -404,6 +404,7 @@ export interface GitProvider extends Disposable { ): Promise; hasCommitBeenPushed(repoPath: string, ref: string): Promise; + hasUnsafeRepositories?(): boolean; isTrackable(uri: Uri): boolean; isTracked(uri: Uri): Promise; diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index ac350be..6fc199d 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -2515,6 +2515,14 @@ export class GitProviderService implements Disposable { return repository.hasUpstreamBranch(); } + @log() + hasUnsafeRepositories(): boolean { + for (const provider of this._providers.values()) { + if (provider.hasUnsafeRepositories?.()) return true; + } + return false; + } + @log({ args: { 0: r => r.uri.toString(true), diff --git a/src/webviews/apps/home/components/plus-banner.ts b/src/webviews/apps/home/components/plus-banner.ts index 37186af..0578b83 100644 --- a/src/webviews/apps/home/components/plus-banner.ts +++ b/src/webviews/apps/home/components/plus-banner.ts @@ -16,7 +16,7 @@ const template = html` ${when( - y => y.extensionEnabled, + y => y.hasRepositories, html`

` private repos.

${when( - y => y.extensionEnabled, + y => y.hasRepositories, html`

` GitLens+ features on private repos.

${when( - y => y.extensionEnabled, + y => y.hasRepositories, html`

` private repos.

${when( - y => y.extensionEnabled, + y => y.hasRepositories, html`

` SubscriptionState.Free, SubscriptionState.FreePreviewTrialExpired, SubscriptionState.FreePlusTrialExpired, - ].includes(x.state) && x.extensionEnabled, + ].includes(x.state) && x.hasRepositories, html`

${when( @@ -152,7 +152,7 @@ const template = html` `, )} ${when( - x => !x.extensionEnabled, + x => !x.hasRepositories, html`

To use GitLens+, open a folder containing a git repository or clone from a URL from the Explorer. @@ -230,7 +230,7 @@ export class PlusBanner extends FASTElement { plus = true; @observable - extensionEnabled = true; + hasRepositories = false; get daysRemaining() { if (this.days < 1) { diff --git a/src/webviews/apps/home/home.ts b/src/webviews/apps/home/home.ts index 8a69858..893fc43 100644 --- a/src/webviews/apps/home/home.ts +++ b/src/webviews/apps/home/home.ts @@ -7,8 +7,8 @@ import type { State } from '../../home/protocol'; import { CompleteStepCommandType, DidChangeConfigurationType, - DidChangeExtensionEnabledType, DidChangeLayoutType, + DidChangeRepositoriesType, DidChangeSubscriptionNotificationType, DismissBannerCommandType, DismissSectionCommandType, @@ -95,11 +95,11 @@ export class HomeApp extends App { this.updateState(); }); break; - case DidChangeExtensionEnabledType.method: + case DidChangeRepositoriesType.method: this.log(`onMessageReceived(${msg.id}): name=${msg.method}`); - onIpc(DidChangeExtensionEnabledType, msg, params => { - this.state.extensionEnabled = params.extensionEnabled; + onIpc(DidChangeRepositoriesType, msg, params => { + this.state.repositories = { ...params }; this.updateNoRepo(); }); break; @@ -242,13 +242,16 @@ export class HomeApp extends App { } private updateNoRepo() { - const { extensionEnabled } = this.state; + const { repositories } = this.state; + const hasRepos = repositories.count > 0; - const value = extensionEnabled ? 'true' : 'false'; + // TODO@d13 provide better feedback if there are unsafe repos (maybe even if there are no "open" repos?) + + const value = hasRepos ? 'true' : 'false'; let $el = document.getElementById('no-repo'); $el?.setAttribute('aria-hidden', value); - if (extensionEnabled) { + if (hasRepos) { $el?.setAttribute('hidden', value); } else { $el?.removeAttribute('hidden'); @@ -256,7 +259,7 @@ export class HomeApp extends App { $el = document.getElementById('no-repo-alert'); $el?.setAttribute('aria-hidden', value); - if (extensionEnabled) { + if (hasRepos) { $el?.setAttribute('hidden', value); } else { $el?.removeAttribute('hidden'); @@ -283,7 +286,7 @@ export class HomeApp extends App { $plusContent.setAttribute('visibility', visibility); $plusContent.setAttribute('plan', subscription.plan.effective.name); $plusContent.setAttribute('plus', plusEnabled.toString()); - ($plusContent as PlusBanner).extensionEnabled = this.state.extensionEnabled; + ($plusContent as PlusBanner).hasRepositories = this.state.repositories.count > 0; } $plusContent = document.getElementById('plus-content'); diff --git a/src/webviews/home/homeWebview.ts b/src/webviews/home/homeWebview.ts index f9f4ed5..486ddc3 100644 --- a/src/webviews/home/homeWebview.ts +++ b/src/webviews/home/homeWebview.ts @@ -8,7 +8,6 @@ import type { SubscriptionChangeEvent } from '../../plus/subscription/subscripti import type { Subscription } from '../../subscription'; import { executeCoreCommand, registerCommand } from '../../system/command'; import { configuration } from '../../system/configuration'; -import { getContext, onDidChangeContext } from '../../system/context'; import type { Deferrable } from '../../system/function'; import { debounce } from '../../system/function'; import type { StorageChangeEvent } from '../../system/storage'; @@ -20,8 +19,8 @@ import { CompletedActions, CompleteStepCommandType, DidChangeConfigurationType, - DidChangeExtensionEnabledType, DidChangeLayoutType, + DidChangeRepositoriesType, DidChangeSubscriptionNotificationType, DismissBannerCommandType, DismissSectionCommandType, @@ -34,10 +33,7 @@ export class HomeWebviewProvider implements WebviewProvider { constructor(private readonly container: Container, private readonly host: WebviewController) { this._disposable = Disposable.from( this.container.subscription.onDidChange(this.onSubscriptionChanged, this), - onDidChangeContext(key => { - if (key !== 'gitlens:disabled') return; - this.notifyExtensionEnabled(); - }), + this.container.git.onDidChangeRepositories(this.onRepositoriesChanged, this), configuration.onDidChange(this.onConfigurationChanged, this), this.container.storage.onDidChange(this.onStorageChanged, this), ); @@ -55,6 +51,10 @@ export class HomeWebviewProvider implements WebviewProvider { this.notifyDidChangeConfiguration(); } + private onRepositoriesChanged() { + this.notifyDidChangeRepositories(); + } + private onStorageChanged(e: StorageChangeEvent) { if (e.key !== 'views:layout') return; @@ -223,7 +223,7 @@ export class HomeWebviewProvider implements WebviewProvider { const dismissedBanners = this.container.storage.get('home:banners:dismissed', []); return { - extensionEnabled: this.getExtensionEnabled(), + repositories: this.getRepositoriesState(), webroot: this.host.getWebRoot(), subscription: sub.subscription, completedActions: sub.completedActions, @@ -250,16 +250,18 @@ export class HomeWebviewProvider implements WebviewProvider { }); } - private getExtensionEnabled() { - return !getContext('gitlens:disabled', false); + private getRepositoriesState() { + return { + count: this.container.git.repositoryCount, + openCount: this.container.git.openRepositoryCount, + hasUnsafe: this.container.git.hasUnsafeRepositories(), + }; } - private notifyExtensionEnabled() { + private notifyDidChangeRepositories() { if (!this.host.ready) return; - void this.host.notify(DidChangeExtensionEnabledType, { - extensionEnabled: this.getExtensionEnabled(), - }); + void this.host.notify(DidChangeRepositoriesType, this.getRepositoriesState()); } private getPlusEnabled() { diff --git a/src/webviews/home/protocol.ts b/src/webviews/home/protocol.ts index 422f3fa..d8c2179 100644 --- a/src/webviews/home/protocol.ts +++ b/src/webviews/home/protocol.ts @@ -9,7 +9,11 @@ export const enum CompletedActions { } export interface State { - extensionEnabled: boolean; + repositories: { + count: number; + openCount: number; + hasUnsafe: boolean; + }; webroot?: string; subscription: Subscription; completedActions: CompletedActions[]; @@ -51,12 +55,12 @@ export const DidChangeSubscriptionNotificationType = new IpcNotificationType( - 'extensionEnabled/didChange', -); +export const DidChangeRepositoriesType = new IpcNotificationType('repositories/didChange'); export interface DidChangeConfigurationParams { plusEnabled: boolean;