diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 00000000000..297165e5681 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,2 @@ +# Automatically request review from desktop/code-reviewers on all PRs +* @desktop/code-reviewers diff --git a/app/package.json b/app/package.json index e2720e31c97..40f4388e84b 100644 --- a/app/package.json +++ b/app/package.json @@ -5,7 +5,7 @@ "productName": "GitHub Desktop Plus", "bundleID": "com.github.GitHubClient", "companyName": "GitHub, Inc.", - "version": "3.5.7-beta2", + "version": "3.5.7", "main": "./main.js", "repository": { "type": "git", diff --git a/app/src/lib/app-state.ts b/app/src/lib/app-state.ts index 80bc0fdbfa7..cf6aef5580b 100644 --- a/app/src/lib/app-state.ts +++ b/app/src/lib/app-state.ts @@ -1,5 +1,6 @@ import { Account } from '../models/account' import { CommitIdentity } from '../models/commit-identity' +import { IConfigValueOrigin } from './git/config' import { IDiff, ImageDiffType } from '../models/diff' import { Repository, ILocalRepositoryState } from '../models/repository' import { Branch, IAheadBehind } from '../models/branch' @@ -321,6 +322,9 @@ export interface IAppState { /** Whether or not the worktrees dropdown should be shown in the toolbar */ readonly showWorktrees: boolean + /** Whether linked worktrees should be shown under their repository in the sidebar */ + readonly showWorktreesInSidebar: boolean + /** Whether or not the Compare tab should be shown in the repository view */ readonly showCompareTab: boolean @@ -363,6 +367,8 @@ export interface IAppState { */ readonly commitSpellcheckEnabled: boolean + readonly showCommitAuthorInfo: boolean + /** * Record of what logged in users have been checked to see if thank you is in * order for external contributions in latest release. @@ -562,6 +568,9 @@ export interface IRepositoryState { */ readonly commitAuthor: CommitIdentity | null + readonly commitAuthorNameOrigin: IConfigValueOrigin | null + readonly commitAuthorEmailOrigin: IConfigValueOrigin | null + readonly branchesState: IBranchesState readonly worktreesState: IWorktreesState diff --git a/app/src/lib/git/config.ts b/app/src/lib/git/config.ts index 08519747d03..b61c414f2db 100644 --- a/app/src/lib/git/config.ts +++ b/app/src/lib/git/config.ts @@ -282,3 +282,111 @@ async function removeConfigValueInPath( await git(flags, path || __dirname, 'removeConfigValueInPath', options) } + +export interface IConfigValueOrigin { + readonly value: string + readonly scope: string + readonly origin: string +} + +/** + * Look up a config value along with its source file and scope. + * Requires Git 2.26+ for --show-scope. + */ +export async function getConfigValueWithOrigin( + repository: Repository, + name: string +): Promise { + const result = await git( + ['config', '--show-origin', '--show-scope', '-z', name], + repository.path, + 'getConfigValueWithOrigin', + // 0 = found, 1 = key not set, 128 = not a git repo or git error + { successExitCodes: new Set([0, 1, 128]) } + ) + + if (result.exitCode !== 0) { + return null + } + + const parts = result.stdout.split('\0') + if (parts.length >= 3) { + return { + scope: parts[0], + origin: parts[1], + value: parts[2], + } + } + + return null +} + +/** + * Extract the file path from a config value origin, stripping the `file:` prefix. + * When repositoryPath is provided, relative paths (e.g. `.git/config` for local + * scope) are resolved to absolute paths. + */ +export function getOriginFilePath( + origin: IConfigValueOrigin, + repositoryPath?: string +): string { + const filePath = origin.origin.replace(/^file:/, '') + // Git returns relative paths for local/worktree scope (e.g. `.git/config`) + if (repositoryPath && !/^([a-zA-Z]:|[/\\])/.test(filePath)) { + const base = repositoryPath.replace(/[\\/]+$/, '') + return `${base}/${filePath}` + } + return filePath +} + +/** + * Check whether a global-scoped config value comes from a conditionally + * included file (via includeIf directive) rather than a standard location. + */ +export function isConditionalInclude(origin: IConfigValueOrigin): boolean { + if (origin.scope !== 'global') { + return false + } + const filePath = getOriginFilePath(origin) + return ( + !/[/\\]\.gitconfig$/i.test(filePath) && + !/[/\\]\.config[/\\]git[/\\]config$/i.test(filePath) + ) +} + +/** Format a human-readable scope description for a config value origin. */ +export function formatConfigScope(origin: IConfigValueOrigin): string { + if (origin.scope === 'local') { + return 'local' + } else if (origin.scope === 'system') { + return 'system' + } else if (origin.scope === 'worktree') { + return 'worktree' + } else if (origin.scope === 'global') { + return isConditionalInclude(origin) ? 'global, via [includeIf]' : 'global' + } + return origin.scope +} + +/** + * Format the file path for a config value origin. + * For local/worktree scope, displays the path with a `` prefix. + */ +export function formatConfigPath( + origin: IConfigValueOrigin, + repositoryPath: string +): string { + const rawPath = origin.origin.replace(/^file:/, '') + if (origin.scope === 'local' || origin.scope === 'worktree') { + // Git returns relative paths for local scope (e.g. `.git/config`) + if (!/^([a-zA-Z]:|[/\\])/.test(rawPath)) { + return '/' + rawPath + } + // Absolute path — strip repo prefix + const normalized = repositoryPath.replace(/[\\/]+$/, '') + if (rawPath.toLowerCase().startsWith(normalized.toLowerCase())) { + return '' + rawPath.slice(normalized.length) + } + } + return rawPath +} diff --git a/app/src/lib/git/worktree.ts b/app/src/lib/git/worktree.ts index 158acbd4f96..2722412fd56 100644 --- a/app/src/lib/git/worktree.ts +++ b/app/src/lib/git/worktree.ts @@ -5,6 +5,15 @@ import type { WorktreeEntry, WorktreeType } from '../../models/worktree' import { git } from './core' import { normalizePath } from '../helpers/path' +function getDotGitPath(repositoryPath: string): string { + return Path.join(repositoryPath, '.git') +} + +export interface IWorktreePathInfo { + readonly isLinkedWorktree: boolean + readonly mainWorktreePath: string | null +} + export function parseWorktreePorcelainOutput( stdout: string ): ReadonlyArray { @@ -104,6 +113,10 @@ export async function removeWorktree( await git(args, repository.path, 'removeWorktree') } +export async function pruneWorktrees(repository: Repository): Promise { + await git(['worktree', 'prune'], repository.path, 'pruneWorktrees') +} + export async function moveWorktree( repository: Repository, oldPath: string, @@ -135,17 +148,48 @@ export async function getMainWorktreePath( return main?.path ?? null } -/** - * Synchronously checks if a repository path is a linked worktree by examining - * whether `.git` is a file (linked worktree) or directory (main worktree). - */ -export function isLinkedWorktreeSync(repositoryPath: string): boolean { +export function getWorktreePathInfoSync( + repositoryPath: string +): IWorktreePathInfo | null { try { - const dotGit = Path.join(repositoryPath, '.git') + const dotGit = getDotGitPath(repositoryPath) // eslint-disable-next-line no-sync const stats = Fs.statSync(dotGit) - return stats.isFile() + + if (stats.isDirectory()) { + return { isLinkedWorktree: false, mainWorktreePath: repositoryPath } + } + + if (!stats.isFile()) { + return null + } + + // eslint-disable-next-line no-sync + const contents = Fs.readFileSync(dotGit, 'utf8').trim() + if (!contents.startsWith('gitdir: ')) { + return null + } + + const gitDirPath = Path.resolve( + repositoryPath, + contents.substring('gitdir: '.length) + ) + + // eslint-disable-next-line no-sync + const commondir = Fs.readFileSync( + Path.join(gitDirPath, 'commondir'), + 'utf8' + ).trim() + if (commondir.length === 0) { + return null + } + + const commonGitDir = Path.resolve(gitDirPath, commondir) + return { + isLinkedWorktree: true, + mainWorktreePath: Path.dirname(commonGitDir), + } } catch { - return false + return null } } diff --git a/app/src/lib/stats/stats-store.ts b/app/src/lib/stats/stats-store.ts index 1480eb46d8c..e8dc4844f76 100644 --- a/app/src/lib/stats/stats-store.ts +++ b/app/src/lib/stats/stats-store.ts @@ -42,6 +42,7 @@ import { getRendererGUID } from '../get-renderer-guid' import { ValidNotificationPullRequestReviewState } from '../valid-notification-pull-request-review' import { useExternalCredentialHelperKey } from '../trampoline/use-external-credential-helper' import { getUserAgent } from '../http' +import { getHooksEnvEnabled } from '../hooks/config' type PullRequestReviewStatFieldInfix = | 'Approved' @@ -428,6 +429,9 @@ interface ICalculatedStats { * Whether or not the user has the filtering changes enabled **/ readonly filteringChangesEnabled: boolean + + /** Whether or not the user has the git hooks environment enabled */ + readonly gitHooksEnvEnabled: boolean } type DailyStats = ICalculatedStats & @@ -646,6 +650,7 @@ export class StatsStore implements IStatsStore { diffCheckMarksVisible, useExternalCredentialHelper, filteringChangesEnabled, + gitHooksEnvEnabled: getHooksEnvEnabled(), } } diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 464440e8456..9e52c62a13b 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -204,6 +204,11 @@ import { } from '../../ui/lib/diff-mode' import { pathExists } from '../../ui/lib/path-exists' import { updateStore } from '../../ui/lib/update-store' +import { + getPreferredWorktreePath, + clearPreferredWorktreePath, +} from '../worktree-preferences' +import { normalizePath } from '../helpers/path' import { resizableComponentClass } from '../../ui/resizable' import { BypassReasonType } from '../../ui/secret-scanning/bypass-push-protection-dialog' import { findContributionTargetDefaultBranch } from '../branch' @@ -301,6 +306,7 @@ import { installLFSHooks, isUsingLFS, } from '../git/lfs' +import { getConfigValueWithOrigin, IConfigValueOrigin } from '../git/config' import { determineMergeability } from '../git/merge-tree' import { listWorktrees } from '../git/worktree' import { reorder } from '../git/reorder' @@ -377,6 +383,13 @@ import { BranchPruner } from './helpers/branch-pruner' import { createTutorialRepository } from './helpers/create-tutorial-repository' import { findRemoteBranchName } from './helpers/find-branch-name' import { RepositoryIndicatorUpdater } from './helpers/repository-indicator-updater' +import { + createSidebarStateFromStatus, + findSidebarWorktreeStateRepository, + getCurrentWorktreeEntryForRepository, + shouldRefreshSidebarWorktrees, + withSidebarWorktrees, +} from './helpers/sidebar-worktrees' import { OnboardingTutorialAssessor } from './helpers/tutorial-assessor' import { getNotificationsEnabled, @@ -390,6 +403,7 @@ import { } from './updates/changes-state' import { updateRemoteUrl } from './updates/update-remote-url' import { getRepoHooks } from '../hooks/get-repo-hooks' +import pLimit from 'p-limit' const LastSelectedRepositoryIDKey = 'last-selected-repository-id' @@ -417,6 +431,7 @@ const branchDropdownWidthConfigKey: string = 'branch-dropdown-width' const defaultWorktreeDropdownWidth: number = 230 const worktreeDropdownWidthConfigKey: string = 'worktree-dropdown-width' +const MaxConcurrentSidebarWorktreePreloads = 4 const defaultPushPullButtonWidth: number = 230 const pushPullButtonWidthConfigKey: string = 'push-pull-button-width' @@ -464,6 +479,9 @@ const hideWhitespaceInPullRequestDiffKey = const commitSpellcheckEnabledDefault = true const commitSpellcheckEnabledKey = 'commit-spellcheck-enabled' +const showCommitAuthorInfoDefault = false +const showCommitAuthorInfoKey = 'show-commit-author-info' + export const tabSizeDefault: number = 4 const tabSizeKey: string = 'tab-size' @@ -471,6 +489,7 @@ const shellKey = 'shell' const showRecentRepositoriesKey = 'show-recent-repositories' const showWorktreesKey = 'show-worktrees' +const showWorktreesInSidebarKey = 'show-worktrees-in-sidebar' const showCompareTabKey = 'show-compare-tab' const showCompareTabDefault = true const repositoryIndicatorsEnabledKey = 'enable-repository-indicators' @@ -610,6 +629,7 @@ export class AppStore extends TypedBaseStore { hideWhitespaceInPullRequestDiffDefault /** Whether or not the spellchecker is enabled for commit summary and description */ private commitSpellcheckEnabled: boolean = commitSpellcheckEnabledDefault + private showCommitAuthorInfo: boolean = showCommitAuthorInfoDefault private showSideBySideDiff: boolean = ShowSideBySideDiffDefault private uncommittedChangesStrategy = defaultUncommittedChangesStrategy @@ -641,6 +661,8 @@ export class AppStore extends TypedBaseStore { private titleBarStyle: TitleBarStyle = 'native' private showRecentRepositories: boolean = true private showWorktrees: boolean = false + private showWorktreesInSidebar: boolean = false + private readonly lastSidebarWorktreeRefreshAt = new Map() private showCompareTab: boolean = showCompareTabDefault private hideWindowOnQuit: boolean = __DARWIN__ @@ -762,6 +784,8 @@ export class AppStore extends TypedBaseStore { this.showRecentRepositories = getBoolean(showRecentRepositoriesKey) ?? true this.showWorktrees = getBoolean(showWorktreesKey) ?? false + this.showWorktreesInSidebar = + this.showWorktrees && (getBoolean(showWorktreesInSidebarKey) ?? false) this.showCompareTab = getBoolean(showCompareTabKey, showCompareTabDefault) this.repositoryIndicatorUpdater = new RepositoryIndicatorUpdater( @@ -919,6 +943,16 @@ export class AppStore extends TypedBaseStore { } } + private pruneSidebarWorktreeRefreshCache() { + const currentRepositoryHashes = new Set(this.repositories.map(r => r.hash)) + + for (const hash of this.lastSidebarWorktreeRefreshAt.keys()) { + if (!currentRepositoryHashes.has(hash)) { + this.lastSidebarWorktreeRefreshAt.delete(hash) + } + } + } + private recordTutorialStepCompleted(step: TutorialStep): void { if (!isValidTutorialStep(step)) { return @@ -1033,7 +1067,11 @@ export class AppStore extends TypedBaseStore { this.repositoriesStore.onDidUpdate(updateRepositories => { this.repositories = updateRepositories + this.pruneSidebarWorktreeRefreshCache() this.updateRepositorySelectionAfterRepositoriesChanged() + if (this.showWorktreesInSidebar) { + void this.preloadSidebarWorktrees() + } this.emitUpdate() }) @@ -1071,10 +1109,12 @@ export class AppStore extends TypedBaseStore { } protected emitUpdate() { - // If the window is hidden then we won't get an animation frame, but there - // may still be work we wanna do in response to the state change. So - // immediately emit the update. - if (this.windowState === 'hidden') { + // If the window is hidden or not focused then we won't reliably get + // animation frames (especially on Windows where Chromium throttles + // requestAnimationFrame for unfocused windows), but there may still be + // work we wanna do in response to the state change. So immediately emit + // the update. + if (this.windowState === 'hidden' || !this.appIsFocused) { this.emitUpdateNow() return } @@ -1215,6 +1255,7 @@ export class AppStore extends TypedBaseStore { titleBarStyle: this.titleBarStyle, showRecentRepositories: this.showRecentRepositories, showWorktrees: this.showWorktrees, + showWorktreesInSidebar: this.showWorktreesInSidebar, showCompareTab: this.showCompareTab, apiRepositories: this.apiRepositoriesStore.getState(), useWindowsOpenSSH: this.useWindowsOpenSSH, @@ -1224,6 +1265,7 @@ export class AppStore extends TypedBaseStore { repositoryIndicatorsEnabled: this.repositoryIndicatorsEnabled, hideWindowOnQuit: this.hideWindowOnQuit, commitSpellcheckEnabled: this.commitSpellcheckEnabled, + showCommitAuthorInfo: this.showCommitAuthorInfo, currentDragElement: this.currentDragElement, lastThankYou: this.lastThankYou, useCustomEditor: this.useCustomEditor, @@ -2134,7 +2176,8 @@ export class AppStore extends TypedBaseStore { /** This shouldn't be called directly. See `Dispatcher`. */ public async _selectRepository( repository: Repository | CloningRepository | null, - persistSelection: boolean = true + persistSelection: boolean = true, + followPreferredWorktree: boolean = true ): Promise { const previouslySelectedRepository = this.selectedRepository @@ -2152,20 +2195,57 @@ export class AppStore extends TypedBaseStore { this.selectedRepository = repository - this.emitUpdate() this.stopBackgroundFetching() this.stopPullRequestUpdater() this._clearBanner() this.stopBackgroundPruner() if (repository == null) { + this.emitUpdate() return Promise.resolve(null) } if (!(repository instanceof Repository)) { + this.emitUpdate() return Promise.resolve(null) } + // When returning to a repository that has worktrees, restore the + // previously active linked worktree so the user doesn't always land + // on the main worktree after switching repos. + if (followPreferredWorktree && !repository.isLinkedWorktree) { + const repoPath = normalizePath(repository.path) + const preferredPath = getPreferredWorktreePath(repoPath) + + if (preferredPath && preferredPath !== repoPath) { + const linkedRepo = this.repositories.find( + r => + r instanceof Repository && normalizePath(r.path) === preferredPath + ) + + if (linkedRepo instanceof Repository) { + repository = linkedRepo + this.selectedRepository = repository + } else { + const exists = await pathExists(preferredPath) + if (exists) { + const addedRepos = await this._addRepositories( + [preferredPath], + repository.login + ) + if (addedRepos.length > 0) { + repository = addedRepos[0] + this.selectedRepository = repository + } + } else { + clearPreferredWorktreePath(repoPath) + } + } + } + } + + this.emitUpdate() + if (persistSelection) { setNumber(LastSelectedRepositoryIDKey, repository.id) } @@ -2461,8 +2541,12 @@ export class AppStore extends TypedBaseStore { this.accounts = accounts this.repositories = repositories + this.pruneSidebarWorktreeRefreshCache() this.updateRepositorySelectionAfterRepositoriesChanged() + if (this.showWorktreesInSidebar) { + void this.preloadSidebarWorktrees() + } this.sidebarWidth = constrain( getNumber(sidebarWidthConfigKey, defaultSidebarWidth) @@ -2590,6 +2674,10 @@ export class AppStore extends TypedBaseStore { commitSpellcheckEnabledKey, commitSpellcheckEnabledDefault ) + this.showCommitAuthorInfo = getBoolean( + showCommitAuthorInfoKey, + showCommitAuthorInfoDefault + ) this.showSideBySideDiff = getShowSideBySideDiff() this.selectedTheme = getPersistedThemeName() @@ -2945,7 +3033,20 @@ export class AppStore extends TypedBaseStore { r.id === selectedRepository.id ) || null - newSelectedRepository = r + if (r !== null) { + newSelectedRepository = r + } else if ( + selectedRepository instanceof Repository && + selectedRepository.id < 0 + ) { + // Synthetic sidebar-only worktree rows are transient selections and + // won't exist in the saved repositories list. Preserve the current + // selection across repository store updates instead of falling back to + // the previously selected saved repository. + newSelectedRepository = selectedRepository + } else { + newSelectedRepository = null + } } if (newSelectedRepository === null && this.repositories.length > 0) { @@ -3959,6 +4060,10 @@ export class AppStore extends TypedBaseStore { const state = this.repositoryStateCache.get(repository) const gitStore = this.gitStoreCache.get(repository) + const sidebarRepository = findSidebarWorktreeStateRepository( + this.repositories, + repository + ) // if we cannot get a valid status it's a good indicator that the repository // is in a bad state - let's mark it as missing here and give up on the @@ -3975,6 +4080,35 @@ export class AppStore extends TypedBaseStore { await gitStore.loadRemotes() await gitStore.loadBranches() await gitStore.loadWorktrees() + this.repositoryStateCache.updateWorktreesState(repository, () => ({ + allWorktrees: gitStore.allWorktrees, + currentWorktree: gitStore.currentWorktree, + })) + if (sidebarRepository !== repository) { + this.repositoryStateCache.updateWorktreesState(sidebarRepository, () => ({ + allWorktrees: gitStore.allWorktrees, + currentWorktree: getCurrentWorktreeEntryForRepository( + gitStore.allWorktrees, + sidebarRepository + ), + })) + } + + if (this.showWorktreesInSidebar) { + this.lastSidebarWorktreeRefreshAt.set(repository.hash, Date.now()) + this.lastSidebarWorktreeRefreshAt.set(sidebarRepository.hash, Date.now()) + this.updateSidebarIndicator(repository, status) + const refreshed = this.localRepositoryStateLookup.get( + sidebarRepository.id + ) + if (refreshed !== undefined) { + this.localRepositoryStateLookup.set( + sidebarRepository.id, + withSidebarWorktrees(refreshed, gitStore.allWorktrees) + ) + } + } + this.emitUpdate() const section = state.selectedSection let refreshSectionPromise: Promise @@ -4026,6 +4160,63 @@ export class AppStore extends TypedBaseStore { } } + private async preloadSidebarWorktrees() { + const limit = pLimit(MaxConcurrentSidebarWorktreePreloads) + + await Promise.all( + this.repositories.map(repository => + limit(async () => { + const exists = await pathExists(repository.path) + if (!exists) { + const existing = this.localRepositoryStateLookup.get(repository.id) + if (existing !== undefined) { + this.localRepositoryStateLookup.set( + repository.id, + withSidebarWorktrees(existing, []) + ) + } + return + } + + try { + const gitStore = this.gitStoreCache.get(repository) + await gitStore.loadWorktrees() + this.repositoryStateCache.updateWorktreesState(repository, () => ({ + allWorktrees: gitStore.allWorktrees, + currentWorktree: gitStore.currentWorktree, + })) + + const existing = this.localRepositoryStateLookup.get(repository.id) + if (existing !== undefined) { + this.localRepositoryStateLookup.set( + repository.id, + withSidebarWorktrees(existing, gitStore.allWorktrees) + ) + } + this.lastSidebarWorktreeRefreshAt.set(repository.hash, Date.now()) + this.emitUpdate() + } catch (error) { + log.warn( + `[AppStore] Failed to preload sidebar worktrees for '${nameOf( + repository + )}'`, + error + ) + const existing = this.localRepositoryStateLookup.get(repository.id) + if (existing !== undefined) { + this.localRepositoryStateLookup.set( + repository.id, + withSidebarWorktrees(existing, []) + ) + } + } + }) + ) + ) + + this.emitUpdate() + } + private async updateStashEntryCountMetric( repository: Repository, desktopStashEntryCount: number, @@ -4049,10 +4240,10 @@ export class AppStore extends TypedBaseStore { /** * Update the repository sidebar indicator for the repository */ - private async updateSidebarIndicator( + private updateSidebarIndicator( repository: Repository, status: IStatusResult | null - ): Promise { + ): void { const lookup = this.localRepositoryStateLookup if (repository.missing) { @@ -4065,18 +4256,26 @@ export class AppStore extends TypedBaseStore { return } - lookup.set(repository.id, { - aheadBehind: status.branchAheadBehind || null, - changedFilesCount: status.workingDirectory.files.length, - branchName: status.currentBranch || null, - defaultBranchName: repository.defaultBranch, - }) + lookup.set( + repository.id, + createSidebarStateFromStatus( + repository, + status, + lookup.get(repository.id), + this.repositoryStateCache.get(repository).worktreesState.allWorktrees, + this.showWorktreesInSidebar + ) + ) } /** * Refresh indicator in repository list for a specific repository */ private refreshIndicatorForRepository = async (repository: Repository) => { const lookup = this.localRepositoryStateLookup + const sidebarRepository = findSidebarWorktreeStateRepository( + this.repositories, + repository + ) if (repository.missing) { lookup.delete(repository.id) @@ -4096,6 +4295,28 @@ export class AppStore extends TypedBaseStore { return } + if ( + this.showWorktreesInSidebar && + shouldRefreshSidebarWorktrees( + this.lastSidebarWorktreeRefreshAt.get(sidebarRepository.hash) + ) + ) { + const sidebarGitStore = this.gitStoreCache.get(sidebarRepository) + await sidebarGitStore.loadWorktrees() + this.repositoryStateCache.updateWorktreesState(sidebarRepository, () => ({ + allWorktrees: sidebarGitStore.allWorktrees, + currentWorktree: sidebarGitStore.currentWorktree, + })) + const refreshed = lookup.get(sidebarRepository.id) + if (refreshed !== undefined) { + lookup.set( + sidebarRepository.id, + withSidebarWorktrees(refreshed, sidebarGitStore.allWorktrees) + ) + } + this.lastSidebarWorktreeRefreshAt.set(sidebarRepository.hash, Date.now()) + } + this.updateSidebarIndicator(repository, status) this.emitUpdate() @@ -4117,6 +4338,7 @@ export class AppStore extends TypedBaseStore { changedFilesCount: existing?.changedFilesCount ?? 0, branchName: existing?.branchName ?? null, defaultBranchName: existing?.defaultBranchName ?? null, + allWorktrees: existing?.allWorktrees ?? [], }) this.emitUpdate() } @@ -4191,10 +4413,33 @@ export class AppStore extends TypedBaseStore { } setBoolean(showWorktreesKey, showWorktrees) this.showWorktrees = showWorktrees + if (!showWorktrees && this.showWorktreesInSidebar) { + setBoolean(showWorktreesInSidebarKey, false) + this.showWorktreesInSidebar = false + this.lastSidebarWorktreeRefreshAt.clear() + } this.updateResizableConstraints() this.emitUpdate() } + public _setShowWorktreesInSidebar(showWorktreesInSidebar: boolean) { + if (this.showWorktreesInSidebar === showWorktreesInSidebar) { + return + } + + if (showWorktreesInSidebar && !this.showWorktrees) { + return + } + + setBoolean(showWorktreesInSidebarKey, showWorktreesInSidebar) + this.showWorktreesInSidebar = showWorktreesInSidebar + this.lastSidebarWorktreeRefreshAt.clear() + if (showWorktreesInSidebar) { + void this.preloadSidebarWorktrees() + } + this.emitUpdate() + } + public _setShowCompareTab(showCompareTab: boolean) { if (this.showCompareTab === showCompareTab) { return @@ -4225,6 +4470,17 @@ export class AppStore extends TypedBaseStore { this.emitUpdate() } + public _setShowCommitAuthorInfo(showCommitAuthorInfo: boolean) { + if (this.showCommitAuthorInfo === showCommitAuthorInfo) { + return + } + + setBoolean(showCommitAuthorInfoKey, showCommitAuthorInfo) + this.showCommitAuthorInfo = showCommitAuthorInfo + + this.emitUpdate() + } + public _setUseWindowsOpenSSH(useWindowsOpenSSH: boolean) { setBoolean(UseWindowsOpenSSHKey, useWindowsOpenSSH) this.useWindowsOpenSSH = useWindowsOpenSSH @@ -4297,8 +4553,22 @@ export class AppStore extends TypedBaseStore { getAuthorIdentity(repository) )) || null + let commitAuthorNameOrigin: IConfigValueOrigin | null = null + let commitAuthorEmailOrigin: IConfigValueOrigin | null = null + + try { + ;[commitAuthorNameOrigin, commitAuthorEmailOrigin] = await Promise.all([ + getConfigValueWithOrigin(repository, 'user.name'), + getConfigValueWithOrigin(repository, 'user.email'), + ]) + } catch (e) { + log.warn('Failed to get config value origins', e) + } + this.repositoryStateCache.update(repository, () => ({ commitAuthor, + commitAuthorNameOrigin, + commitAuthorEmailOrigin, })) this.emitUpdate() } @@ -7162,11 +7432,29 @@ export class AppStore extends TypedBaseStore { continue } - const addedRepo = await this.repositoriesStore.addRepository( + let addedRepo = await this.repositoriesStore.addRepository( validatedPath, login ) + // When a linked worktree is added as a standalone repository and the + // main worktree is already known to Desktop, inherit that GitHub + // association up front so the saved row lands in the same top-level + // group after restart. + const mainWorktreeRepo = addedRepo.isLinkedWorktree + ? matchExistingRepository(repositories, addedRepo.mainWorktreePath) + : undefined + + if ( + mainWorktreeRepo !== undefined && + isRepositoryWithGitHubRepository(mainWorktreeRepo) + ) { + addedRepo = await this.repositoriesStore.setGitHubRepository( + addedRepo, + mainWorktreeRepo.gitHubRepository + ) + } + // initialize the remotes for this new repository to ensure it can fetch // it's GitHub-related details using the GitHub API (if applicable) const gitStore = this.gitStoreCache.get(addedRepo) @@ -7232,6 +7520,23 @@ export class AppStore extends TypedBaseStore { return } + if (repository instanceof Repository) { + if (repository.isLinkedWorktree) { + const repoPath = normalizePath(repository.path) + const mainRepo = this.repositories.find( + r => + r instanceof Repository && + !r.isLinkedWorktree && + getPreferredWorktreePath(normalizePath(r.path)) === repoPath + ) + if (mainRepo instanceof Repository) { + clearPreferredWorktreePath(normalizePath(mainRepo.path)) + } + } else { + clearPreferredWorktreePath(normalizePath(repository.path)) + } + } + const allRepositories = await this.repositoriesStore.getAll() if (allRepositories.length === 0) { this._closeFoldout(FoldoutType.Repository) diff --git a/app/src/lib/stores/helpers/sidebar-worktrees.ts b/app/src/lib/stores/helpers/sidebar-worktrees.ts new file mode 100644 index 00000000000..49f68e722da --- /dev/null +++ b/app/src/lib/stores/helpers/sidebar-worktrees.ts @@ -0,0 +1,75 @@ +import { IStatusResult } from '../../git' +import { normalizePath } from '../../helpers/path' +import { ILocalRepositoryState, Repository } from '../../../models/repository' +import { WorktreeEntry } from '../../../models/worktree' + +/** + * Refresh sidebar worktree metadata more sparingly than the repository + * indicator loop to avoid repeatedly shelling out to `git worktree list`. + */ +export const SidebarWorktreeRefreshInterval = 2 * 60 * 1000 + +export function findSidebarWorktreeStateRepository( + repositories: ReadonlyArray, + repository: Repository +) { + if (!repository.isLinkedWorktree) { + return repository + } + + const mainWorktreePath = normalizePath(repository.mainWorktreePath) + return ( + repositories.find( + candidate => normalizePath(candidate.path) === mainWorktreePath + ) ?? repository + ) +} + +export function getCurrentWorktreeEntryForRepository( + allWorktrees: ReadonlyArray, + repository: Repository +) { + return ( + allWorktrees.find( + worktree => + normalizePath(worktree.path) === normalizePath(repository.path) + ) ?? null + ) +} + +export function createSidebarStateFromStatus( + repository: Repository, + status: IStatusResult, + existing: ILocalRepositoryState | undefined, + allWorktrees: ReadonlyArray, + showWorktreesInSidebar: boolean +): ILocalRepositoryState { + return { + aheadBehind: status.branchAheadBehind || null, + changedFilesCount: status.workingDirectory.files.length, + branchName: status.currentBranch || null, + defaultBranchName: repository.defaultBranch, + allWorktrees: showWorktreesInSidebar ? allWorktrees : [], + } +} + +export function withSidebarWorktrees( + existing: ILocalRepositoryState, + allWorktrees: ReadonlyArray +): ILocalRepositoryState { + return { + ...existing, + allWorktrees, + } +} + +export function shouldRefreshSidebarWorktrees( + lastRefreshedAt: number | undefined, + now: number = Date.now() +) { + if (lastRefreshedAt === undefined) { + return true + } + + return now - lastRefreshedAt >= SidebarWorktreeRefreshInterval +} diff --git a/app/src/lib/stores/repositories-store.ts b/app/src/lib/stores/repositories-store.ts index eb4a98e49f5..9d4d598b05b 100644 --- a/app/src/lib/stores/repositories-store.ts +++ b/app/src/lib/stores/repositories-store.ts @@ -479,6 +479,12 @@ export class RepositoriesStore extends TypedBaseStore< repository: Repository, date: number = Date.now() ): Promise { + // Synthetic sidebar-only worktree rows are transient repositories that + // are not persisted in the repositories store. + if (repository.id < 0) { + return + } + await this.db.repositories.update(repository.id, { lastStashCheckDate: date, }) @@ -496,6 +502,12 @@ export class RepositoriesStore extends TypedBaseStore< public async getLastStashCheckDate( repository: Repository ): Promise { + // Synthetic sidebar-only worktree rows are transient repositories that + // are not persisted in the repositories store. + if (repository.id < 0) { + return null + } + let lastCheckDate = this.lastStashCheckCache.get(repository.id) || null if (lastCheckDate !== null) { return lastCheckDate diff --git a/app/src/lib/stores/repository-state-cache.ts b/app/src/lib/stores/repository-state-cache.ts index 51c1f02fbf4..e5c33a4b872 100644 --- a/app/src/lib/stores/repository-state-cache.ts +++ b/app/src/lib/stores/repository-state-cache.ts @@ -375,6 +375,8 @@ function getInitialRepositoryState(): IRepositoryState { }, pullRequestState: null, commitAuthor: null, + commitAuthorNameOrigin: null, + commitAuthorEmailOrigin: null, commitLookup: new Map(), localCommitSHAs: [], localTags: null, diff --git a/app/src/lib/worktree-preferences.ts b/app/src/lib/worktree-preferences.ts new file mode 100644 index 00000000000..a8ee2124811 --- /dev/null +++ b/app/src/lib/worktree-preferences.ts @@ -0,0 +1,59 @@ +import { normalizePath } from './helpers/path' + +const StorageKey = 'worktree-active-paths' + +function getPreferences(): Record { + try { + const raw = localStorage.getItem(StorageKey) + return raw ? JSON.parse(raw) : {} + } catch { + return {} + } +} + +function savePreferences(prefs: Record) { + localStorage.setItem(StorageKey, JSON.stringify(prefs)) +} + +/** + * Get the preferred worktree path for a given main worktree path. + * Returns null if no preference is stored (defaults to main worktree). + */ +export function getPreferredWorktreePath( + mainWorktreePath: string +): string | null { + const prefs = getPreferences() + return prefs[normalizePath(mainWorktreePath)] ?? null +} + +/** + * Store the user's active worktree choice for a repository. + * If the active path is the main worktree itself, the preference is cleared + * so the repo defaults to its main worktree on next visit. + */ +export function setPreferredWorktreePath( + mainWorktreePath: string, + activeWorktreePath: string +) { + const prefs = getPreferences() + const normalizedMain = normalizePath(mainWorktreePath) + const normalizedActive = normalizePath(activeWorktreePath) + + if (normalizedMain === normalizedActive) { + delete prefs[normalizedMain] + } else { + prefs[normalizedMain] = normalizedActive + } + + savePreferences(prefs) +} + +/** + * Clear any stored worktree preference for a repository so it + * defaults to the main worktree on next visit. + */ +export function clearPreferredWorktreePath(mainWorktreePath: string) { + const prefs = getPreferences() + delete prefs[normalizePath(mainWorktreePath)] + savePreferences(prefs) +} diff --git a/app/src/models/popup.ts b/app/src/models/popup.ts index 549d6bfb737..50a25c18f54 100644 --- a/app/src/models/popup.ts +++ b/app/src/models/popup.ts @@ -513,5 +513,7 @@ export type PopupDetail = type: PopupType.DeleteWorktree repository: Repository worktreePath: string + storedRepositoryToRemove?: Repository + isDeletingCurrentWorktree?: boolean } export type Popup = IBasePopup & PopupDetail diff --git a/app/src/models/repository.ts b/app/src/models/repository.ts index e855665eb8a..2430b12b7f7 100644 --- a/app/src/models/repository.ts +++ b/app/src/models/repository.ts @@ -2,13 +2,14 @@ import * as Path from 'path' import { GitHubRepository, ForkedGitHubRepository } from './github-repository' import { IAheadBehind } from './branch' +import { WorktreeEntry } from './worktree' import { WorkflowPreferences, ForkContributionTarget, } from './workflow-preferences' import { assertNever, fatalError } from '../lib/fatal-error' import { createEqualityHash } from './equality-hash' -import { isLinkedWorktreeSync } from '../lib/git/worktree' +import { getWorktreePathInfoSync } from '../lib/git/worktree' import { getRemotes } from '../lib/git' import { findDefaultRemote } from '../lib/stores/helpers/find-default-remote' import { isTrustedRemoteHost } from '../lib/api' @@ -56,7 +57,9 @@ export class Repository { */ private _url: string | null = null + private _hasLoadedWorktreeInfo = false private _isLinkedWorktree: boolean | undefined = undefined + private _mainWorktreePath: string | undefined = undefined /** * @param path The working directory of this repository @@ -98,15 +101,29 @@ export class Repository { ) } + private ensureWorktreeInfoLoaded() { + if (this._hasLoadedWorktreeInfo) { + return + } + + const worktreeInfo = getWorktreePathInfoSync(this.path) + this._isLinkedWorktree = worktreeInfo?.isLinkedWorktree ?? false + this._mainWorktreePath = worktreeInfo?.mainWorktreePath ?? this.path + this._hasLoadedWorktreeInfo = true + } + public get path(): string { return this.mainWorkTree.path } public get isLinkedWorktree(): boolean { - if (this._isLinkedWorktree === undefined) { - this._isLinkedWorktree = isLinkedWorktreeSync(this.path) - } - return this._isLinkedWorktree + this.ensureWorktreeInfoLoaded() + return this._isLinkedWorktree ?? false + } + + public get mainWorktreePath(): string { + this.ensureWorktreeInfoLoaded() + return this._mainWorktreePath ?? this.path } public get url(): string | null { @@ -229,6 +246,10 @@ export interface ILocalRepositoryState { * The name of the default branch, or `undefined` if not available. */ readonly defaultBranchName: string | null + /** + * All worktrees known for this repository. + */ + readonly allWorktrees: ReadonlyArray } /** diff --git a/app/src/ui/add-repository/add-existing-repository.tsx b/app/src/ui/add-repository/add-existing-repository.tsx index f17e24fb217..a70ef1540b1 100644 --- a/app/src/ui/add-repository/add-existing-repository.tsx +++ b/app/src/ui/add-repository/add-existing-repository.tsx @@ -77,7 +77,9 @@ export class AddExistingRepository extends React.Component< } private async updatePath(path: string) { - this.setState({ path }) + await new Promise(resolve => { + this.setState({ path }, resolve) + }) } private async validatePath(path: string): Promise { @@ -89,7 +91,8 @@ export class AddExistingRepository extends React.Component< return false } - const type = await getRepositoryType(path) + const resolvedPath = this.resolvedPath(path) + const type = await getRepositoryType(resolvedPath) const isRepository = type.kind !== 'missing' && type.kind !== 'unsafe' const isRepositoryUnsafe = type.kind === 'unsafe' diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index d4f651c457a..d134e83e3f9 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -1672,6 +1672,7 @@ export class App extends React.Component { selectedExternalEditor={this.state.selectedExternalEditor} useWindowsOpenSSH={this.state.useWindowsOpenSSH} showCommitLengthWarning={this.state.showCommitLengthWarning} + showCommitAuthorInfo={this.state.showCommitAuthorInfo} notificationsEnabled={this.state.notificationsEnabled} optOutOfUsageTracking={this.state.optOutOfUsageTracking} useExternalCredentialHelper={this.state.useExternalCredentialHelper} @@ -1688,6 +1689,7 @@ export class App extends React.Component { titleBarStyle={this.state.titleBarStyle} showRecentRepositories={this.state.showRecentRepositories} showWorktrees={this.state.showWorktrees} + showWorktreesInSidebar={this.state.showWorktreesInSidebar} showCompareTab={this.state.showCompareTab} repositoryIndicatorsEnabled={this.state.repositoryIndicatorsEnabled} hideWindowOnQuit={this.state.hideWindowOnQuit} @@ -2761,6 +2763,8 @@ export class App extends React.Component { key="delete-worktree" repository={popup.repository} worktreePath={popup.worktreePath} + storedRepositoryToRemove={popup.storedRepositoryToRemove} + isDeletingCurrentWorktree={popup.isDeletingCurrentWorktree} dispatcher={this.props.dispatcher} onDismissed={onPopupDismissedFn} /> @@ -3070,9 +3074,14 @@ export class App extends React.Component { const { useCustomShell, selectedShell } = this.state const filterText = this.state.repositoryFilterText - const repositories = this.state.repositories.filter( - r => !(r instanceof Repository && r.isLinkedWorktree) - ) + const repositories = this.state.showWorktreesInSidebar + ? [...this.state.repositories] + : this.state.repositories.filter( + r => !(r instanceof Repository && r.isLinkedWorktree) + ) + const localRepositoryStateLookup = this.state.showWorktreesInSidebar + ? new Map(this.state.localRepositoryStateLookup) + : this.state.localRepositoryStateLookup return ( { repositories={repositories} recentRepositories={this.state.recentRepositories} showRecentRepositories={this.state.showRecentRepositories} - localRepositoryStateLookup={this.state.localRepositoryStateLookup} + localRepositoryStateLookup={localRepositoryStateLookup} askForConfirmationOnRemoveRepository={ this.state.askForConfirmationOnRepositoryRemoval } @@ -3096,6 +3105,7 @@ export class App extends React.Component { shellLabel={useCustomShell ? undefined : selectedShell} dispatcher={this.props.dispatcher} showBranchNameInRepoList={this.state.showBranchNameInRepoList} + showWorktreesInSidebar={this.state.showWorktreesInSidebar} /> ) } @@ -3735,6 +3745,7 @@ export class App extends React.Component { aheadBehindStore={this.props.aheadBehindStore} commitSpellcheckEnabled={this.state.commitSpellcheckEnabled} showCommitLengthWarning={this.state.showCommitLengthWarning} + showCommitAuthorInfo={this.state.showCommitAuthorInfo} onCherryPick={this.startCherryPickWithoutBranch} pullRequestSuggestedNextAction={state.pullRequestSuggestedNextAction} showChangesFilter={state.showChangesFilter} @@ -3818,7 +3829,7 @@ export class App extends React.Component { } private onSelectionChanged = (repository: Repository | CloningRepository) => { - this.props.dispatcher.selectRepository(repository) + this.props.dispatcher.selectRepository(repository, true, false) this.props.dispatcher.closeFoldout(FoldoutType.Repository) } diff --git a/app/src/ui/changes/commit-message.tsx b/app/src/ui/changes/commit-message.tsx index f97c64985cf..95417684d53 100644 --- a/app/src/ui/changes/commit-message.tsx +++ b/app/src/ui/changes/commit-message.tsx @@ -37,7 +37,14 @@ import { isAttributableEmailFor, lookupPreferredEmail, } from '../../lib/email' -import { setGlobalConfigValue } from '../../lib/git/config' +import { + setGlobalConfigValue, + IConfigValueOrigin, + getOriginFilePath, + isConditionalInclude, + formatConfigScope, + formatConfigPath, +} from '../../lib/git/config' import { Popup, PopupType } from '../../models/popup' import { RepositorySettingsTab } from '../repository-settings/repository-settings' import { IdealSummaryLength } from '../../lib/wrap-rich-text-commit-message' @@ -67,9 +74,66 @@ import { enableHooksEnvironment, } from '../../lib/feature-flag' import { AriaLiveContainer } from '../accessibility/aria-live-container' +import { TooltippedContent } from '../lib/tooltipped-content' +import { showItemInFolder } from '../main-process-proxy' import { HookProgress } from '../../lib/git' import { assertNever } from '../../lib/fatal-error' +function renderScopeValue(origin: IConfigValueOrigin): JSX.Element { + if (isConditionalInclude(origin)) { + return ( + + global, via [includeIf] + + ) + } + return {formatConfigScope(origin)} +} + +function formatConfigOriginTooltip( + fieldName: string, + origin: IConfigValueOrigin, + repositoryPath: string, + onRevealFile: () => void, + warningMessage?: string, + warningAccountLogin?: string, + onOpenRepositoryRemoteSettings?: () => void +): JSX.Element { + return ( +
+ {warningMessage && warningAccountLogin && ( + <> + + + + + {warningMessage} + + + + Repository is linked to @{warningAccountLogin} + + + + Change account in{' '} + + repository settings + + + + )} + {fieldName}: + {origin.value} + Scope: + {renderScopeValue(origin)} + File: + + {formatConfigPath(origin, repositoryPath)} + +
+ ) +} + const addAuthorIcon: OcticonSymbolVariant = { w: 18, h: 13, @@ -240,6 +304,10 @@ interface ICommitMessageProps { repository: Repository, options: Partial ) => void + + readonly commitAuthorNameOrigin?: IConfigValueOrigin | null + readonly commitAuthorEmailOrigin?: IConfigValueOrigin | null + readonly showCommitAuthorInfo?: boolean } interface ICommitMessageState { @@ -770,7 +838,7 @@ export class CommitMessage extends React.Component< } } - return ( + const avatar = ( ) + + if (!this.props.showCommitAuthorInfo || !commitAuthor) { + return avatar + } + + const { commitAuthorNameOrigin, commitAuthorEmailOrigin } = this.props + const repoPath = this.props.repository.path + const isMisattributed = warningType === 'misattribution' + const warningMessage = isMisattributed + ? 'Email does not match linked account' + : undefined + const warningAccountLogin = isMisattributed + ? repositoryAccount?.login + : undefined + const nameTooltip = commitAuthorNameOrigin + ? formatConfigOriginTooltip( + 'Name', + commitAuthorNameOrigin, + repoPath, + this.onRevealNameConfigFile + ) + : undefined + const emailTooltip = commitAuthorEmailOrigin + ? formatConfigOriginTooltip( + 'Email', + commitAuthorEmailOrigin, + repoPath, + this.onRevealEmailConfigFile, + warningMessage, + warningAccountLogin, + this.onOpenRepositoryRemoteSettings + ) + : undefined + + const identityClasses = classNames('commit-author-identity', { + warning: isMisattributed, + }) + const emailClasses = classNames('commit-author-email', { + warning: isMisattributed, + }) + + return ( +
+ {avatar} +
+ + {commitAuthor.name} + + + {commitAuthor.email} + +
+
+ ) + } + + private onRevealNameConfigFile = () => { + const { commitAuthorNameOrigin } = this.props + if (commitAuthorNameOrigin) { + showItemInFolder( + getOriginFilePath(commitAuthorNameOrigin, this.props.repository.path) + ) + } + } + + private onRevealEmailConfigFile = () => { + const { commitAuthorEmailOrigin } = this.props + if (commitAuthorEmailOrigin) { + showItemInFolder( + getOriginFilePath(commitAuthorEmailOrigin, this.props.repository.path) + ) + } } private onUpdateUserEmail = async (email: string) => { @@ -806,6 +956,14 @@ export class CommitMessage extends React.Component< }) } + private onOpenRepositoryRemoteSettings = () => { + this.props.onShowPopup({ + type: PopupType.RepositorySettings, + repository: this.props.repository, + initialSelectedTab: RepositorySettingsTab.Remote, + }) + } + private onOpenGitSettings = () => { this.props.onShowPopup({ type: PopupType.Preferences, @@ -1738,9 +1896,12 @@ export class CommitMessage extends React.Component< onContextMenu={this.onContextMenu} ref={this.wrapperRef} > -
- {this.renderAvatar()} + {/* When showing author info, avatar is rendered above the summary + row as part of the identity block. Otherwise, inline in summary. */} + {this.props.showCommitAuthorInfo && this.renderAvatar()} +
+ {!this.props.showCommitAuthorInfo && this.renderAvatar()} /** The file list filter state containing all filter options */ @@ -1027,6 +1032,9 @@ export class FilterChangesList extends React.Component< branch={this.props.branch} mostRecentLocalCommit={this.props.mostRecentLocalCommit} commitAuthor={this.props.commitAuthor} + commitAuthorNameOrigin={this.props.commitAuthorNameOrigin} + commitAuthorEmailOrigin={this.props.commitAuthorEmailOrigin} + showCommitAuthorInfo={this.props.showCommitAuthorInfo} isShowingModal={this.props.isShowingModal} isShowingFoldout={this.props.isShowingFoldout} anyFilesSelected={anyFilesSelected} diff --git a/app/src/ui/changes/sidebar.tsx b/app/src/ui/changes/sidebar.tsx index 4a25c175ec5..ca03b488cf4 100644 --- a/app/src/ui/changes/sidebar.tsx +++ b/app/src/ui/changes/sidebar.tsx @@ -13,6 +13,7 @@ import { Repository } from '../../models/repository' import { Dispatcher } from '../dispatcher' import { IssuesStore, GitHubUserStore } from '../../lib/stores' import { CommitIdentity } from '../../models/commit-identity' +import { IConfigValueOrigin } from '../../lib/git/config' import { Commit, ICommitContext } from '../../models/commit' import { UndoCommit } from './undo-commit' import { @@ -48,6 +49,8 @@ interface IChangesSidebarProps { readonly aheadBehind: IAheadBehind | null readonly dispatcher: Dispatcher readonly commitAuthor: CommitIdentity | null + readonly commitAuthorNameOrigin?: IConfigValueOrigin | null + readonly commitAuthorEmailOrigin?: IConfigValueOrigin | null readonly branch: string | null readonly emoji: Map readonly mostRecentLocalCommit: Commit | null @@ -94,6 +97,8 @@ interface IChangesSidebarProps { readonly showCommitLengthWarning: boolean + readonly showCommitAuthorInfo: boolean + /** Whether or not to show the changes filter */ readonly showChangesFilter: boolean @@ -460,6 +465,9 @@ export class ChangesSidebar extends React.Component { onOpenItem={this.onOpenItem} onRowClick={this.onChangedItemClick} commitAuthor={this.props.commitAuthor} + commitAuthorNameOrigin={this.props.commitAuthorNameOrigin} + commitAuthorEmailOrigin={this.props.commitAuthorEmailOrigin} + showCommitAuthorInfo={this.props.showCommitAuthorInfo} branch={this.props.branch} commitMessage={commitMessage} focusCommitMessage={this.props.focusCommitMessage} diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index f970c1c1ffa..010e187f218 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -314,9 +314,14 @@ export class Dispatcher { /** Select the repository. */ public selectRepository( repository: Repository | CloningRepository, - persistSelection: boolean = true + persistSelection: boolean = true, + followPreferredWorktree: boolean = true ): Promise { - return this.appStore._selectRepository(repository, persistSelection) + return this.appStore._selectRepository( + repository, + persistSelection, + followPreferredWorktree + ) } /** Change the selected section in the repository. */ @@ -2932,6 +2937,10 @@ export class Dispatcher { this.appStore._setShowWorktrees(showWorktrees) } + public setShowWorktreesInSidebar(showWorktreesInSidebar: boolean) { + this.appStore._setShowWorktreesInSidebar(showWorktreesInSidebar) + } + public setShowCompareTab(showCompareTab: boolean) { this.appStore._setShowCompareTab(showCompareTab) } @@ -2944,6 +2953,10 @@ export class Dispatcher { this.appStore._setCommitSpellcheckEnabled(commitSpellcheckEnabled) } + public setShowCommitAuthorInfo(showCommitAuthorInfo: boolean) { + this.appStore._setShowCommitAuthorInfo(showCommitAuthorInfo) + } + public setUseWindowsOpenSSH(useWindowsOpenSSH: boolean) { this.appStore._setUseWindowsOpenSSH(useWindowsOpenSSH) } diff --git a/app/src/ui/lib/install-cli.ts b/app/src/ui/lib/install-cli.ts index 7d95b4b62fe..73fca5db6c5 100644 --- a/app/src/ui/lib/install-cli.ts +++ b/app/src/ui/lib/install-cli.ts @@ -4,10 +4,14 @@ import * as fsAdmin from 'fs-admin-forked' import { mkdir, readlink, symlink, unlink } from 'fs/promises' /** The path for the installed command line tool. */ -export const InstalledCLIPath = '/usr/local/bin/github' +export const InstalledCLIPath = '/usr/local/bin/github-desktop-plus-cli' /** The path to the packaged CLI. */ -const PackagedPath = Path.resolve(__dirname, 'static', 'github.sh') +const PackagedPath = Path.resolve( + __dirname, + 'static', + 'github-desktop-plus-cli.sh' +) /** Install the command line tool on macOS. */ export async function installCLI(): Promise { diff --git a/app/src/ui/preferences/accessibility.tsx b/app/src/ui/preferences/accessibility.tsx index 3f64caab997..02251d95f7c 100644 --- a/app/src/ui/preferences/accessibility.tsx +++ b/app/src/ui/preferences/accessibility.tsx @@ -21,7 +21,7 @@ export class Accessibility extends React.Component< public render() { return ( -
+

Accessibility

void readonly showWorktrees: boolean readonly onShowWorktreesChanged: (show: boolean) => void + readonly showWorktreesInSidebar: boolean + readonly onShowWorktreesInSidebarChanged: (show: boolean) => void readonly showCompareTab: boolean readonly onShowCompareTabChanged: (show: boolean) => void readonly showBranchNameInRepoList: ShowBranchNameInRepoListSetting @@ -47,6 +49,7 @@ interface IAppearanceState { readonly titleBarStyle: TitleBarStyle readonly showRecentRepositories: boolean readonly showWorktrees: boolean + readonly showWorktreesInSidebar: boolean readonly showCompareTab: boolean } @@ -76,6 +79,7 @@ export class Appearance extends React.Component< titleBarStyle: props.titleBarStyle, showRecentRepositories: props.showRecentRepositories, showWorktrees: props.showWorktrees, + showWorktreesInSidebar: props.showWorktreesInSidebar, showCompareTab: props.showCompareTab, } @@ -85,7 +89,13 @@ export class Appearance extends React.Component< } public async componentDidUpdate(prevProps: IAppearanceProps) { - if (prevProps === this.props) { + if ( + prevProps.selectedTheme === this.props.selectedTheme && + prevProps.selectedTabSize === this.props.selectedTabSize && + prevProps.showWorktrees === this.props.showWorktrees && + prevProps.showWorktreesInSidebar === this.props.showWorktreesInSidebar && + prevProps.showCompareTab === this.props.showCompareTab + ) { return } @@ -99,7 +109,13 @@ export class Appearance extends React.Component< const selectedTabSize = this.props.selectedTabSize - this.setState({ selectedTheme, selectedTabSize }) + this.setState({ + selectedTheme, + selectedTabSize, + showWorktrees: this.props.showWorktrees, + showWorktreesInSidebar: this.props.showWorktreesInSidebar, + showCompareTab: this.props.showCompareTab, + }) } private initializeSelectedTheme = async () => { @@ -124,7 +140,10 @@ export class Appearance extends React.Component< event: React.FormEvent ) => { const show = event.currentTarget.checked - this.setState({ showWorktrees: show }) + this.setState({ + showWorktrees: show, + showWorktreesInSidebar: show ? this.state.showWorktreesInSidebar : false, + }) this.props.onShowWorktreesChanged(show) } @@ -136,6 +155,14 @@ export class Appearance extends React.Component< this.props.onShowCompareTabChanged(show) } + private onShowWorktreesInSidebarChanged = ( + event: React.FormEvent + ) => { + const show = event.currentTarget.checked + this.setState({ showWorktreesInSidebar: show }) + this.props.onShowWorktreesInSidebarChanged(show) + } + private onSelectedTabSizeChanged = ( event: React.FormEvent ) => { @@ -363,6 +390,17 @@ export class Appearance extends React.Component< } onChange={this.onShowWorktreesChanged} /> + {this.state.showWorktrees && ( + + )}

{'Commit list'}

diff --git a/app/src/ui/preferences/git.tsx b/app/src/ui/preferences/git.tsx index 964cfab5bb7..7ee041154f7 100644 --- a/app/src/ui/preferences/git.tsx +++ b/app/src/ui/preferences/git.tsx @@ -37,6 +37,13 @@ interface IGitProps { readonly enableGitHookEnv: boolean readonly cacheGitHookEnv: boolean readonly selectedShell: string + + readonly showCommitAuthorInfo: boolean + readonly onShowCommitAuthorInfoChanged: (show: boolean) => void + + readonly setGlobalAuthor: boolean + readonly globalAuthorWasSet: boolean + readonly onSetGlobalAuthorChanged: (value: boolean) => void } const windowsShells: ReadonlyArray = [ @@ -76,14 +83,6 @@ export class Git extends React.Component { private renderHooksSettings() { return ( <> -
- GitHub Desktop hook support is experimental and currently only - supports hooks related to committing. Please{' '} - - let us know - {' '} - if you encounter any issues or have feedback! -
{ } onChange={this.onEnableGitHookEnvChanged} /> -

+

When enabled, GitHub Desktop will attempt to load environment variables from your shell when executing Git hooks. This is useful if your Git hooks depend on environment variables set in your shell @@ -132,7 +131,10 @@ export class Git extends React.Component { } /> -

+
Cache hook environment variables to improve performance. Disable if your hooks rely on frequently changing environment variables.
@@ -151,9 +153,7 @@ export class Git extends React.Component { > Author Default branch - - Hooks Beta - + Hooks
{this.renderCurrentTab()}
@@ -172,9 +172,37 @@ export class Git extends React.Component { return null } + private onSetGlobalAuthorChanged = ( + event: React.FormEvent + ) => { + this.props.onSetGlobalAuthorChanged(event.currentTarget.checked) + } + + private onShowCommitAuthorInfoChanged = ( + event: React.FormEvent + ) => { + this.props.onShowCommitAuthorInfoChanged(event.currentTarget.checked) + } + private renderGitConfigAuthorInfo() { return ( <> +

Global Author

+ + {!this.props.setGlobalAuthor && this.props.globalAuthorWasSet && ( +
+ ⚠️ + Saving will remove user.name and user.email from your global Git + config. Make sure your repositories have local config or includeIf + rules set up, otherwise commits may fail. +
+ )} { accounts={this.props.accounts} onEmailChanged={this.props.onEmailChanged} onNameChanged={this.props.onNameChanged} + disabled={!this.props.setGlobalAuthor} /> {this.renderEditGlobalGitConfigInfo()} +

Commit Identity Display

+ +

+ Git resolves author identity from multiple config files with different + priorities.{' '} + + Learn more about config scopes + + . +

) } diff --git a/app/src/ui/preferences/preferences.tsx b/app/src/ui/preferences/preferences.tsx index a2368e5dc9e..9400b6cf986 100644 --- a/app/src/ui/preferences/preferences.tsx +++ b/app/src/ui/preferences/preferences.tsx @@ -11,6 +11,7 @@ import { Dialog, DialogFooter, DialogError } from '../dialog' import { getGlobalConfigValue, setGlobalConfigValue, + removeGlobalConfigValue, } from '../../lib/git/config' import { lookupPreferredEmail } from '../../lib/email' import { Shell, getAvailableShells } from '../../lib/shells' @@ -73,6 +74,7 @@ interface IPreferencesProps { readonly onDismissed: () => void readonly useWindowsOpenSSH: boolean readonly showCommitLengthWarning: boolean + readonly showCommitAuthorInfo: boolean readonly notificationsEnabled: boolean readonly optOutOfUsageTracking: boolean readonly useExternalCredentialHelper: boolean @@ -99,6 +101,7 @@ interface IPreferencesProps { readonly titleBarStyle: TitleBarStyle readonly showRecentRepositories: boolean readonly showWorktrees: boolean + readonly showWorktreesInSidebar: boolean readonly showCompareTab: boolean readonly repositoryIndicatorsEnabled: boolean readonly showBranchNameInRepoList: ShowBranchNameInRepoListSetting @@ -119,9 +122,11 @@ interface IPreferencesState { readonly initialCommitterName: string | null readonly initialCommitterEmail: string | null readonly initialDefaultBranch: string | null + readonly setGlobalAuthor: boolean readonly disallowedCharactersMessage: string | null readonly useWindowsOpenSSH: boolean readonly showCommitLengthWarning: boolean + readonly showCommitAuthorInfo: boolean readonly notificationsEnabled: boolean readonly optOutOfUsageTracking: boolean readonly useExternalCredentialHelper: boolean @@ -147,6 +152,7 @@ interface IPreferencesState { readonly titleBarStyle: TitleBarStyle readonly showRecentRepositories: boolean readonly showWorktrees: boolean + readonly showWorktreesInSidebar: boolean readonly showCompareTab: boolean /** * If unable to save Git configuration values (name, email) @@ -206,6 +212,7 @@ export class Preferences extends React.Component< initialCommitterName: null, initialCommitterEmail: null, initialDefaultBranch: null, + setGlobalAuthor: false, disallowedCharactersMessage: null, availableEditors: [], useCustomEditor: this.props.useCustomEditor, @@ -216,6 +223,7 @@ export class Preferences extends React.Component< this.props.branchPresetScript ?? DefaultCustomIntegration, useWindowsOpenSSH: false, showCommitLengthWarning: false, + showCommitAuthorInfo: false, notificationsEnabled: true, optOutOfUsageTracking: false, useExternalCredentialHelper: false, @@ -235,6 +243,7 @@ export class Preferences extends React.Component< titleBarStyle: this.props.titleBarStyle, showRecentRepositories: this.props.showRecentRepositories, showWorktrees: this.props.showWorktrees, + showWorktreesInSidebar: this.props.showWorktreesInSidebar, showCompareTab: this.props.showCompareTab, repositoryIndicatorsEnabled: this.props.repositoryIndicatorsEnabled, showBranchNameInRepoList: this.props.showBranchNameInRepoList, @@ -296,8 +305,10 @@ export class Preferences extends React.Component< initialCommitterName, initialCommitterEmail, initialDefaultBranch, + setGlobalAuthor: !!initialCommitterName || !!initialCommitterEmail, useWindowsOpenSSH: this.props.useWindowsOpenSSH, showCommitLengthWarning: this.props.showCommitLengthWarning, + showCommitAuthorInfo: this.props.showCommitAuthorInfo, notificationsEnabled: this.props.notificationsEnabled, optOutOfUsageTracking: this.props.optOutOfUsageTracking, useExternalCredentialHelper: this.props.useExternalCredentialHelper, @@ -557,6 +568,14 @@ export class Preferences extends React.Component< selectedShell={ this.state.selectedGitHookEnvShell ?? defaultGitHookEnvShell } + showCommitAuthorInfo={this.state.showCommitAuthorInfo} + onShowCommitAuthorInfoChanged={this.onShowCommitAuthorInfoChanged} + setGlobalAuthor={this.state.setGlobalAuthor} + globalAuthorWasSet={ + !!this.state.initialCommitterName || + !!this.state.initialCommitterEmail + } + onSetGlobalAuthorChanged={this.onSetGlobalAuthorChanged} /> ) @@ -577,6 +596,10 @@ export class Preferences extends React.Component< } showWorktrees={this.state.showWorktrees} onShowWorktreesChanged={this.onShowWorktreesChanged} + showWorktreesInSidebar={this.state.showWorktreesInSidebar} + onShowWorktreesInSidebarChanged={ + this.onShowWorktreesInSidebarChanged + } showCompareTab={this.state.showCompareTab} onShowCompareTabChanged={this.onShowCompareTabChanged} showBranchNameInRepoList={this.state.showBranchNameInRepoList} @@ -719,6 +742,14 @@ export class Preferences extends React.Component< this.setState({ showCommitLengthWarning }) } + private onShowCommitAuthorInfoChanged = (showCommitAuthorInfo: boolean) => { + this.setState({ showCommitAuthorInfo }) + } + + private onSetGlobalAuthorChanged = (setGlobalAuthor: boolean) => { + this.setState({ setGlobalAuthor }) + } + private onNotificationsEnabledChanged = (notificationsEnabled: boolean) => { this.setState({ notificationsEnabled }) } @@ -869,7 +900,18 @@ export class Preferences extends React.Component< } private onShowWorktreesChanged = (showWorktrees: boolean) => { - this.setState({ showWorktrees }) + this.setState(state => ({ + showWorktrees, + showWorktreesInSidebar: showWorktrees + ? state.showWorktreesInSidebar + : false, + })) + } + + private onShowWorktreesInSidebarChanged = ( + showWorktreesInSidebar: boolean + ) => { + this.setState({ showWorktreesInSidebar }) } private onShowCompareTabChanged = (showCompareTab: boolean) => { @@ -895,13 +937,28 @@ export class Preferences extends React.Component< try { let shouldRefreshAuthor = false - if (this.state.committerName !== this.state.initialCommitterName) { - await setGlobalConfigValue('user.name', this.state.committerName) - shouldRefreshAuthor = true - } + if (this.state.setGlobalAuthor) { + if (this.state.committerName !== this.state.initialCommitterName) { + await setGlobalConfigValue('user.name', this.state.committerName) + shouldRefreshAuthor = true + } - if (this.state.committerEmail !== this.state.initialCommitterEmail) { - await setGlobalConfigValue('user.email', this.state.committerEmail) + if (this.state.committerEmail !== this.state.initialCommitterEmail) { + await setGlobalConfigValue('user.email', this.state.committerEmail) + shouldRefreshAuthor = true + } + } else if ( + this.state.initialCommitterName || + this.state.initialCommitterEmail + ) { + // User unchecked the box — remove identity from global config. + // Ignore errors if values are already absent. + try { + await removeGlobalConfigValue('user.name') + } catch {} + try { + await removeGlobalConfigValue('user.email') + } catch {} shouldRefreshAuthor = true } @@ -942,6 +999,12 @@ export class Preferences extends React.Component< dispatcher.setShowWorktrees(this.state.showWorktrees) } + if ( + this.state.showWorktreesInSidebar !== this.props.showWorktreesInSidebar + ) { + dispatcher.setShowWorktreesInSidebar(this.state.showWorktreesInSidebar) + } + if (this.state.showCompareTab !== this.props.showCompareTab) { dispatcher.setShowCompareTab(this.state.showCompareTab) } @@ -983,6 +1046,7 @@ export class Preferences extends React.Component< dispatcher.setUseWindowsOpenSSH(this.state.useWindowsOpenSSH) dispatcher.setShowCommitLengthWarning(this.state.showCommitLengthWarning) + dispatcher.setShowCommitAuthorInfo(this.state.showCommitAuthorInfo) dispatcher.setNotificationsEnabled(this.state.notificationsEnabled) await dispatcher.setStatsOptOut(this.state.optOutOfUsageTracking, false) diff --git a/app/src/ui/repositories-list/group-repositories.ts b/app/src/ui/repositories-list/group-repositories.ts index d2d4d4d83a8..9bf342fd918 100644 --- a/app/src/ui/repositories-list/group-repositories.ts +++ b/app/src/ui/repositories-list/group-repositories.ts @@ -1,18 +1,22 @@ import { Repository, ILocalRepositoryState, - nameOf, isRepositoryWithGitHubRepository, RepositoryWithGitHubRepository, } from '../../models/repository' import { CloningRepository } from '../../models/cloning-repository' import { getHTMLURL } from '../../lib/api' -import { caseInsensitiveCompare, compare } from '../../lib/compare' +import { compare } from '../../lib/compare' import { IFilterListGroup, IFilterListItem } from '../lib/filter-list' import { IAheadBehind } from '../../models/branch' import { assertNever } from '../../lib/fatal-error' import { isGHE, isGHES } from '../../lib/endpoint-capabilities' import { Owner } from '../../models/owner' +import { normalizePath } from '../../lib/helpers/path' +import { + getRepositoryListTitle, + toSortedRepositoryListItems, +} from './worktree-list-items' export type RepositoryListGroup = ( | { @@ -57,12 +61,23 @@ export type Repositoryish = Repository | CloningRepository export interface IRepositoryListItem extends IFilterListItem { readonly text: ReadonlyArray readonly id: string + readonly title: string readonly repository: Repositoryish readonly needsDisambiguation: boolean readonly aheadBehind: IAheadBehind | null readonly changedFilesCount: number readonly branchName: string | null readonly defaultBranchName: string | null + readonly isNestedWorktree: boolean + readonly mainWorktreeName: string | null + readonly isVirtualLinkedWorktree: boolean + readonly isPrunableWorktree: boolean + readonly worktreePath: string | null + readonly sourceRepository: Repository | null +} + +interface IGroupRepositoriesOptions { + readonly showWorktreesInSidebar?: boolean } const recentRepositoriesThreshold = 7 @@ -97,11 +112,24 @@ type RepoGroupItem = { group: RepositoryListGroup; repos: Repositoryish[] } export function groupRepositories( repositories: ReadonlyArray, localRepositoryStateLookup: ReadonlyMap, - recentRepositories: ReadonlyArray + recentRepositories: ReadonlyArray, + options: IGroupRepositoriesOptions = {} ): ReadonlyArray> { const includeRecentGroup = repositories.length > recentRepositoriesThreshold const recentSet = includeRecentGroup ? new Set(recentRepositories) : undefined const groups = new Map() + const repositoryByPath = new Map() + const storedRepositoryPaths = new Set() + + for (const repository of repositories) { + if (!(repository instanceof Repository)) { + continue + } + + const normalizedPath = normalizePath(repository.path) + repositoryByPath.set(normalizedPath, repository) + storedRepositoryPaths.add(normalizedPath) + } const addToGroup = (group: RepositoryListGroup, repo: Repositoryish) => { const key = getGroupKey(group) @@ -130,22 +158,24 @@ export function groupRepositories( group, repos, localRepositoryStateLookup, - groups + groups, + repositoryByPath, + storedRepositoryPaths, + options ), })) } -// Returns the display title for a repository, which is either the alias -// (if available) or the name. -const getDisplayTitle = (r: Repositoryish) => - r instanceof Repository && r.alias != null ? r.alias : r.name - const toSortedListItems = ( group: RepositoryListGroup, repositories: ReadonlyArray, localRepositoryStateLookup: ReadonlyMap, - groups: Map + groups: Map, + repositoryByPath: ReadonlyMap, + storedRepositoryPaths: ReadonlySet, + options: IGroupRepositoriesOptions ): IRepositoryListItem[] => { + const showWorktreesInSidebar = options.showWorktreesInSidebar ?? false const groupNames = new Map() const allNames = new Map() @@ -156,39 +186,23 @@ const toSortedListItems = ( continue } - for (const title of groupItem.repos.map(getDisplayTitle)) { + for (const title of groupItem.repos.map(repo => + getRepositoryListTitle(repo, showWorktreesInSidebar) + )) { allNames.set(title, (allNames.get(title) ?? 0) + 1) if (groupItem.group === group) { groupNames.set(title, (groupNames.get(title) ?? 0) + 1) } } } - - return repositories - .map(r => { - const repoState = localRepositoryStateLookup.get(r.id) - const title = getDisplayTitle(r) - - return { - text: r instanceof Repository ? [title, nameOf(r)] : [title], - id: r.id.toString(), - repository: r, - needsDisambiguation: - // If the repository is in the enterprise group and has a duplicate - // name in the group, we need to disambiguate it. We don't have to - // disambiguate repositories in the 'dotcom' group because they are - // already grouped by owner. If the repository is in the 'recent' - // group and has a duplicate name in any group, we need to - // disambiguate it. - ((groupNames.get(title) ?? 0) > 1 && group.kind === 'enterprise') || - ((allNames.get(title) ?? 0) > 1 && group.kind === 'recent'), - aheadBehind: repoState?.aheadBehind ?? null, - changedFilesCount: repoState?.changedFilesCount ?? 0, - branchName: repoState?.branchName ?? null, - defaultBranchName: repoState?.defaultBranchName ?? null, - } - }) - .sort(({ repository: x }, { repository: y }) => - caseInsensitiveCompare(getDisplayTitle(x), getDisplayTitle(y)) - ) + return toSortedRepositoryListItems({ + group, + repositories, + localRepositoryStateLookup, + groupNames, + allNames, + repositoryByPath, + storedRepositoryPaths, + showWorktreesInSidebar, + }) } diff --git a/app/src/ui/repositories-list/repositories-list.tsx b/app/src/ui/repositories-list/repositories-list.tsx index 256a4785036..c396f5140c2 100644 --- a/app/src/ui/repositories-list/repositories-list.tsx +++ b/app/src/ui/repositories-list/repositories-list.tsx @@ -27,6 +27,11 @@ import { SectionFilterList } from '../lib/section-filter-list' import { assertNever } from '../../lib/fatal-error' import { IAheadBehind } from '../../models/branch' import { ShowBranchNameInRepoListSetting } from '../../models/show-branch-name-in-repo-list' +import { normalizePath } from '../../lib/helpers/path' +import { ClickSource } from '../lib/list' +import { getRepositoryType } from '../../lib/git/rev-parse' +import { FoldoutType } from '../../lib/app-state' +import { pruneWorktrees } from '../../lib/git/worktree' const BlankSlateImage = encodePathAsUrl(__dirname, 'static/empty-no-repo.svg') @@ -82,6 +87,7 @@ interface IRepositoriesListProps { /** Controls when to show the branch name next to each repository */ readonly showBranchNameInRepoList: ShowBranchNameInRepoListSetting + readonly showWorktreesInSidebar: boolean } interface IRepositoriesListState { @@ -103,9 +109,14 @@ function findMatchingListItem( selectedRepository: Repositoryish | null ) { if (selectedRepository !== null) { + const selectedPath = normalizePath(selectedRepository.path) for (const group of groups) { for (const item of group.items) { - if (item.repository.id === selectedRepository.id) { + if ( + item.repository.id === selectedRepository.id || + (item.worktreePath !== null && + normalizePath(item.worktreePath) === selectedPath) + ) { return item } } @@ -115,6 +126,36 @@ function findMatchingListItem( return null } +function isPullableRepository( + repository: Repositoryish, + repositories: ReadonlyArray +): repository is Repository { + if (!(repository instanceof Repository)) { + return false + } + + if (!repository.isLinkedWorktree) { + return true + } + + const mainWorktreePath = normalizePath(repository.mainWorktreePath) + const candidatesWithSameMain = repositories.filter( + (candidate): candidate is Repository => + candidate instanceof Repository && + normalizePath(candidate.mainWorktreePath) === mainWorktreePath + ) + + if (candidatesWithSameMain.length === 0) { + return false + } + + const preferred = + candidatesWithSameMain.find(candidate => !candidate.isLinkedWorktree) ?? + candidatesWithSameMain[0] + + return preferred.id === repository.id +} + /** The list of user-added repositories. */ export class RepositoriesList extends React.Component< IRepositoriesListProps, @@ -130,14 +171,16 @@ export class RepositoriesList extends React.Component< ( repositories: ReadonlyArray | null, localRepositoryStateLookup: ReadonlyMap, - recentRepositories: ReadonlyArray + recentRepositories: ReadonlyArray, + showWorktreesInSidebar: boolean ) => repositories === null ? [] : groupRepositories( repositories, localRepositoryStateLookup, - recentRepositories + recentRepositories, + { showWorktreesInSidebar } ) ) @@ -183,13 +226,17 @@ export class RepositoriesList extends React.Component< const repository = item.repository return ( ) } @@ -228,6 +275,9 @@ export class RepositoriesList extends React.Component< const uncommittedChangesTooltip = hasChanges ? `There are uncommitted changes in this repository.` : null + const prunableWorktreeTooltip = item.isPrunableWorktree + ? 'This worktree entry is stale and should be pruned.' + : null const ahead = aheadBehind?.ahead ?? 0 const behind = aheadBehind?.behind ?? 0 @@ -270,6 +320,16 @@ export class RepositoriesList extends React.Component< {uncommittedChangesTooltip}
)} + {prunableWorktreeTooltip && ( +
+
+ + + +
+ {prunableWorktreeTooltip} +
+ )}
) } @@ -310,7 +370,31 @@ export class RepositoriesList extends React.Component< ) } - private onItemClick = (item: IRepositoryListItem) => { + private onItemClick = (item: IRepositoryListItem, source: ClickSource) => { + if ( + source.kind === 'mouseclick' && + (source.event.button === 2 || + (__DARWIN__ && source.event.button === 0 && source.event.ctrlKey)) + ) { + return + } + + if (item.isPrunableWorktree) { + void this.props.dispatcher.postError( + new Error( + 'This worktree entry is stale. Use the context menu to prune stale worktrees.' + ) + ) + return + } + + if (item.isVirtualLinkedWorktree && item.worktreePath !== null) { + void this.onVirtualWorktreeClick(item).catch(error => + this.props.dispatcher.postError(error) + ) + return + } + const hasIndicator = item.changedFilesCount > 0 || (item.aheadBehind !== null @@ -320,20 +404,107 @@ export class RepositoriesList extends React.Component< this.props.onSelectionChanged(item.repository) } + private onVirtualWorktreeClick = async (item: IRepositoryListItem) => { + if ( + item.worktreePath === null || + item.sourceRepository === null || + !(item.repository instanceof Repository) + ) { + return + } + + const { worktreePath } = item + const existingRepo = this.props.repositories.find( + r => + r instanceof Repository && + normalizePath(r.path) === normalizePath(worktreePath) + ) + + if (existingRepo instanceof Repository) { + await this.props.dispatcher.selectRepository(existingRepo) + await this.props.dispatcher.closeFoldout(FoldoutType.Repository) + return + } + + const repositoryType = await getRepositoryType(worktreePath) + if (repositoryType.kind !== 'regular') { + throw new Error(`${worktreePath} isn't a Git repository.`) + } + + await this.props.dispatcher.selectRepository(item.repository, false) + await this.props.dispatcher.closeFoldout(FoldoutType.Repository) + } + + private onRemoveLinkedWorktree = (item: IRepositoryListItem) => { + const worktreePath = + item.worktreePath ?? + (item.repository instanceof Repository ? item.repository.path : null) + if (worktreePath === null) { + return + } + + const repository = + item.isVirtualLinkedWorktree && item.sourceRepository !== null + ? item.sourceRepository + : item.repository instanceof Repository + ? item.repository + : null + const storedRepositoryToRemove = + item.repository instanceof Repository && !item.isVirtualLinkedWorktree + ? item.repository + : undefined + + if (repository === null) { + return + } + + this.props.dispatcher.showPopup({ + type: PopupType.DeleteWorktree, + repository, + worktreePath, + storedRepositoryToRemove, + isDeletingCurrentWorktree: + this.props.selectedRepository !== null && + normalizePath(this.props.selectedRepository.path) === + normalizePath(worktreePath), + }) + } + + private onPruneStaleWorktrees = async (item: IRepositoryListItem) => { + const repository = + item.sourceRepository ?? + (item.repository instanceof Repository ? item.repository : null) + + if (repository === null) { + return + } + + await pruneWorktrees(repository) + await this.props.dispatcher.refreshRepository(repository) + } + private onItemContextMenu = ( item: IRepositoryListItem, event: React.MouseEvent ) => { event.preventDefault() + event.stopPropagation() const items = generateRepositoryListContextMenu({ onRemoveRepository: this.props.onRemoveRepository, + onRemoveLinkedWorktree: () => this.onRemoveLinkedWorktree(item), onShowRepository: this.props.onShowRepository, onOpenInNewWindow: this.props.onOpenInNewWindow, onOpenInShell: this.props.onOpenInShell, onOpenInExternalEditor: this.props.onOpenInExternalEditor, askForConfirmationOnRemoveRepository: this.props.askForConfirmationOnRemoveRepository, + isLinkedWorktreeRow: + item.isVirtualLinkedWorktree || + (item.repository instanceof Repository && + item.repository.isLinkedWorktree), + isVirtualLinkedWorktreeRow: item.isVirtualLinkedWorktree, + isPrunableWorktreeRow: item.isPrunableWorktree, externalEditorLabel: this.props.externalEditorLabel, onChangeRepositoryAlias: this.onChangeRepositoryAlias, onRemoveRepositoryAlias: this.onRemoveRepositoryAlias, @@ -343,6 +514,11 @@ export class RepositoriesList extends React.Component< repository: item.repository, shellLabel: this.props.shellLabel, onCopyRepoPath: path => this.props.dispatcher.copyPathToClipboard(path), + onPruneStaleWorktrees: () => { + void this.onPruneStaleWorktrees(item).catch(error => + this.props.dispatcher.postError(error) + ) + }, }) showContextualMenu(items) @@ -362,7 +538,8 @@ export class RepositoriesList extends React.Component< let groups = this.getRepositoryGroups( this.props.repositories, this.props.localRepositoryStateLookup, - this.props.recentRepositories + this.props.recentRepositories, + this.props.showWorktreesInSidebar ) if (!this.props.showRecentRepositories) { @@ -506,14 +683,20 @@ export class RepositoriesList extends React.Component< private onPullRepositoriesButtonClick = async () => { this.setState({ pullingRepositories: true }) try { + const repositoriesToPull = this.props.repositories.filter(repository => + isPullableRepository(repository, this.props.repositories) + ) + await Promise.all( - this.props.repositories - .filter(r => r instanceof Repository) - .map(r => - this.props.dispatcher.pull(r).catch(e => { - throw Error(`Error pulling '${r.name}' (${r.path}):\n${e}`, e) - }) - ) + repositoriesToPull.map(repository => + this.props.dispatcher.pull(repository).catch(e => { + const message = e instanceof Error ? e.message : String(e) + throw new Error( + `Error pulling '${repository.name}' (${repository.path}): ${message}`, + { cause: e } + ) + }) + ) ) } catch (e) { this.props.dispatcher.postError(e) diff --git a/app/src/ui/repositories-list/repository-list-item-context-menu.ts b/app/src/ui/repositories-list/repository-list-item-context-menu.ts index e0b2dd41fd0..d212e46c3da 100644 --- a/app/src/ui/repositories-list/repository-list-item-context-menu.ts +++ b/app/src/ui/repositories-list/repository-list-item-context-menu.ts @@ -18,12 +18,17 @@ interface IRepositoryListItemContextMenuConfig { shellLabel: string | undefined externalEditorLabel: string | undefined askForConfirmationOnRemoveRepository: boolean + readonly isLinkedWorktreeRow?: boolean + readonly isVirtualLinkedWorktreeRow?: boolean + readonly isPrunableWorktreeRow?: boolean onViewInBrowser: (repository: Repositoryish) => void onOpenInNewWindow?: (repository: Repositoryish) => void onOpenInShell: (repository: Repositoryish) => void onShowRepository: (repository: Repositoryish) => void onOpenInExternalEditor: (repository: Repositoryish) => void onRemoveRepository: (repository: Repositoryish) => void + onRemoveLinkedWorktree?: () => void + onPruneStaleWorktrees?: () => void onChangeRepositoryAlias: (repository: Repository) => void onRemoveRepositoryAlias: (repository: Repository) => void onChangeRepositoryGroupName: (repository: Repository) => void @@ -35,6 +40,11 @@ export const generateRepositoryListContextMenu = ( config: IRepositoryListItemContextMenuConfig ) => { const { repository } = config + const isLinkedWorktreeRow = config.isLinkedWorktreeRow ?? false + const isPrunableWorktreeRow = config.isPrunableWorktreeRow ?? false + const aliasMenuItems = buildAliasMenuItems(config) + const groupNameMenuItems = buildGroupNameMenuItems(config) + const identityMenuItems = [...aliasMenuItems, ...groupNameMenuItems] const missing = repository instanceof Repository && repository.missing const isGitHub = repository instanceof Repository && @@ -51,9 +61,8 @@ export const generateRepositoryListContextMenu = ( : DefaultShellLabel const items: ReadonlyArray = [ - ...buildAliasMenuItems(config), - ...buildGroupNameMenuItems(config), - { type: 'separator' }, + ...identityMenuItems, + ...(identityMenuItems.length > 0 ? [{ type: 'separator' as const }] : []), { label: __DARWIN__ ? 'Copy Repo Name' : 'Copy repo name', action: () => clipboard.writeText(repository.name), @@ -95,11 +104,39 @@ export const generateRepositoryListContextMenu = ( action: () => config.onOpenInExternalEditor(repository), enabled: !missing, }, - { type: 'separator' }, - { - label: config.askForConfirmationOnRemoveRepository ? 'Remove…' : 'Remove', - action: () => config.onRemoveRepository(repository), - }, + ...(isPrunableWorktreeRow && config.onPruneStaleWorktrees !== undefined + ? [ + { type: 'separator' as const }, + { + label: __DARWIN__ + ? 'Prune Stale Worktrees' + : 'Prune stale worktrees', + action: config.onPruneStaleWorktrees, + }, + ] + : []), + ...(!(isPrunableWorktreeRow && isLinkedWorktreeRow) + ? [ + { type: 'separator' as const }, + { + label: isPrunableWorktreeRow + ? config.askForConfirmationOnRemoveRepository + ? 'Remove…' + : 'Remove' + : isLinkedWorktreeRow + ? 'Delete…' + : config.askForConfirmationOnRemoveRepository + ? 'Remove…' + : 'Remove', + action: + !isPrunableWorktreeRow && + isLinkedWorktreeRow && + config.onRemoveLinkedWorktree !== undefined + ? config.onRemoveLinkedWorktree + : () => config.onRemoveRepository(repository), + }, + ] + : []), ] return items @@ -123,7 +160,10 @@ const buildAliasMenuItems = ( ): ReadonlyArray => { const { repository } = config - if (!(repository instanceof Repository)) { + if ( + !(repository instanceof Repository) || + config.isVirtualLinkedWorktreeRow + ) { return [] } @@ -150,7 +190,11 @@ const buildGroupNameMenuItems = ( ): ReadonlyArray => { const { repository } = config - if (!(repository instanceof Repository)) { + if ( + !(repository instanceof Repository) || + config.isLinkedWorktreeRow || + config.isVirtualLinkedWorktreeRow + ) { return [] } diff --git a/app/src/ui/repositories-list/repository-list-item.tsx b/app/src/ui/repositories-list/repository-list-item.tsx index ed6adf7d4bb..2fbf964ebac 100644 --- a/app/src/ui/repositories-list/repository-list-item.tsx +++ b/app/src/ui/repositories-list/repository-list-item.tsx @@ -14,6 +14,7 @@ import { enableAccessibleListToolTips } from '../../lib/feature-flag' import { TooltippedContent } from '../lib/tooltipped-content' interface IRepositoryListItemProps { + readonly title: string readonly repository: Repositoryish /** Does the repository need to be disambiguated in the list? */ @@ -30,6 +31,9 @@ interface IRepositoryListItemProps { /** The name of the current branch, if it should be displayed */ readonly branchName: string | null + readonly isNestedWorktree: boolean + readonly mainWorktreeName: string | null + readonly isPrunableWorktree: boolean } /** A repository item. */ @@ -44,6 +48,9 @@ export class RepositoryListItem extends React.Component< const gitHubRepo = repository instanceof Repository ? repository.gitHubRepository : null const hasChanges = this.props.changedFilesCount > 0 + const icon = this.props.isNestedWorktree + ? octicons.fileDirectory + : iconForRepository(repository) const alias: string | null = repository instanceof Repository ? repository.alias : null @@ -54,11 +61,19 @@ export class RepositoryListItem extends React.Component< } const classNameList = classNames('name', { - alias: alias !== null, + alias: + alias !== null && + !this.props.isNestedWorktree && + this.props.title === alias, }) return ( -
+
- + -
+
{prefix ? {prefix} : null}
@@ -85,7 +97,7 @@ export class RepositoryListItem extends React.Component< {this.props.branchName} )} - + {this.props.isPrunableWorktree && renderPrunableIndicator()} {repository instanceof Repository && renderRepoIndicators({ aheadBehind: this.props.aheadBehind, @@ -109,6 +121,12 @@ export class RepositoryListItem extends React.Component<
{repo.path}
{this.props.branchName &&
Branch: {this.props.branchName}
} + {this.props.mainWorktreeName && ( +
Repository: {this.props.mainWorktreeName}
+ )} + {this.props.isPrunableWorktree && ( +
This worktree entry is stale and should be pruned.
+ )} ) } @@ -121,7 +139,14 @@ export class RepositoryListItem extends React.Component< return ( nextProps.repository.id !== this.props.repository.id || nextProps.matches !== this.props.matches || - nextProps.branchName !== this.props.branchName + nextProps.title !== this.props.title || + nextProps.needsDisambiguation !== this.props.needsDisambiguation || + nextProps.branchName !== this.props.branchName || + nextProps.aheadBehind !== this.props.aheadBehind || + nextProps.changedFilesCount !== this.props.changedFilesCount || + nextProps.isNestedWorktree !== this.props.isNestedWorktree || + nextProps.mainWorktreeName !== this.props.mainWorktreeName || + nextProps.isPrunableWorktree !== this.props.isPrunableWorktree ) } else { return true @@ -179,5 +204,17 @@ const renderChangesIndicator = () => { ) } +const renderPrunableIndicator = () => { + return ( + + + + ) +} + export const commitGrammar = (commitNum: number) => `${commitNum} commit${commitNum > 1 ? 's' : ''}` // english is hard diff --git a/app/src/ui/repositories-list/worktree-list-items.ts b/app/src/ui/repositories-list/worktree-list-items.ts new file mode 100644 index 00000000000..e0eb32dbc31 --- /dev/null +++ b/app/src/ui/repositories-list/worktree-list-items.ts @@ -0,0 +1,327 @@ +import * as Path from 'path' + +import { + Repository, + ILocalRepositoryState, + nameOf, +} from '../../models/repository' +import { caseInsensitiveCompare } from '../../lib/compare' +import { normalizePath } from '../../lib/helpers/path' +import type { IAheadBehind } from '../../models/branch' +import type { WorktreeEntry } from '../../models/worktree' +import type { + IRepositoryListItem, + RepositoryListGroup, + Repositoryish, +} from './group-repositories' + +let nextVirtualRepositoryId = -1 +const virtualRepositoryIdsByPath = new Map() + +export const getDisplayTitle = (repository: Repositoryish) => + repository instanceof Repository && repository.alias != null + ? repository.alias + : repository.name + +const getLinkedWorktreeDisplayTitle = ( + repository: Repositoryish, + worktreePath?: string +) => + repository instanceof Repository && repository.alias != null + ? repository.alias + : Path.basename(worktreePath ?? repository.path) + +export const getRepositoryListTitle = ( + repository: Repositoryish, + showWorktreesInSidebar: boolean +) => + showWorktreesInSidebar && + repository instanceof Repository && + repository.isLinkedWorktree + ? getLinkedWorktreeDisplayTitle(repository) + : getDisplayTitle(repository) + +const getVirtualRepositoryId = (worktreePath: string) => { + const normalizedPath = normalizePath(worktreePath) + const existingId = virtualRepositoryIdsByPath.get(normalizedPath) + if (existingId !== undefined) { + return existingId + } + + const id = nextVirtualRepositoryId-- + virtualRepositoryIdsByPath.set(normalizedPath, id) + return id +} + +const pruneVirtualRepositoryIds = ( + storedRepositoryPaths: ReadonlySet, + localRepositoryStateLookup: ReadonlyMap +) => { + const knownWorktreePaths = new Set(storedRepositoryPaths) + + for (const state of localRepositoryStateLookup.values()) { + for (const worktree of state.allWorktrees) { + knownWorktreePaths.add(normalizePath(worktree.path)) + } + } + + for (const worktreePath of virtualRepositoryIdsByPath.keys()) { + if (!knownWorktreePaths.has(worktreePath)) { + virtualRepositoryIdsByPath.delete(worktreePath) + } + } +} + +const getBranchNameForWorktree = (worktree: WorktreeEntry) => + worktree.branch?.replace(/^refs\/heads\//, '') ?? null + +const getWorktreeEntryForPath = ( + allWorktrees: ReadonlyArray, + worktreePath: string +) => + allWorktrees.find( + worktree => normalizePath(worktree.path) === normalizePath(worktreePath) + ) ?? null + +interface IToListItemOptions { + readonly isVirtualLinkedWorktree?: boolean + readonly worktreePath?: string + readonly sourceRepository?: Repository | null + readonly branchName?: string | null + readonly changedFilesCount?: number + readonly aheadBehind?: IAheadBehind | null +} + +interface IToSortedListItemsOptions { + readonly group: RepositoryListGroup + readonly repositories: ReadonlyArray + readonly localRepositoryStateLookup: ReadonlyMap< + number, + ILocalRepositoryState + > + readonly groupNames: ReadonlyMap + readonly allNames: ReadonlyMap + readonly repositoryByPath: ReadonlyMap + readonly storedRepositoryPaths: ReadonlySet + readonly showWorktreesInSidebar: boolean +} + +export function toSortedRepositoryListItems({ + group, + repositories, + localRepositoryStateLookup, + groupNames, + allNames, + repositoryByPath, + storedRepositoryPaths, + showWorktreesInSidebar, +}: IToSortedListItemsOptions): IRepositoryListItem[] { + pruneVirtualRepositoryIds(storedRepositoryPaths, localRepositoryStateLookup) + + const toListItem = ( + repository: Repositoryish, + isNestedWorktree: boolean, + options?: IToListItemOptions + ): IRepositoryListItem => { + const repoState = localRepositoryStateLookup.get(repository.id) + const isVirtualLinkedWorktree = options?.isVirtualLinkedWorktree ?? false + const isLinkedWorktree = + !isVirtualLinkedWorktree && + repository instanceof Repository && + repository.isLinkedWorktree + const worktreePath = options?.worktreePath ?? repository.path + const parentRepository = + options?.sourceRepository ?? + (repository instanceof Repository && isLinkedWorktree + ? repositoryByPath.get(normalizePath(repository.mainWorktreePath)) ?? + null + : null) + const parentRepoState = + parentRepository !== null + ? localRepositoryStateLookup.get(parentRepository.id) + : null + const startupWorktreeEntry = + (isLinkedWorktree || isVirtualLinkedWorktree) && parentRepoState != null + ? getWorktreeEntryForPath(parentRepoState.allWorktrees, worktreePath) + : null + const title = + isLinkedWorktree || isVirtualLinkedWorktree + ? getLinkedWorktreeDisplayTitle(repository, worktreePath) + : getDisplayTitle(repository) + const defaultBranchName = + repoState?.defaultBranchName ?? + options?.sourceRepository?.defaultBranch ?? + (repository instanceof Repository ? repository.defaultBranch : null) + const mainWorktreePath = + isVirtualLinkedWorktree && options?.sourceRepository != null + ? options.sourceRepository.mainWorktreePath + : repository instanceof Repository + ? repository.mainWorktreePath + : options?.sourceRepository?.mainWorktreePath ?? repository.path + const mainWorktreeName = + (isLinkedWorktree || isVirtualLinkedWorktree) && isNestedWorktree + ? Path.basename(mainWorktreePath) + : null + + return { + text: + repository instanceof Repository + ? isLinkedWorktree || isVirtualLinkedWorktree + ? [title, nameOf(repository), Path.basename(mainWorktreePath)] + : [title, nameOf(repository)] + : [title], + title, + id: options?.worktreePath + ? `worktree:${normalizePath(options.worktreePath)}` + : repository.id.toString(), + repository, + needsDisambiguation: + ((groupNames.get(title) ?? 0) > 1 && group.kind === 'enterprise') || + ((allNames.get(title) ?? 0) > 1 && group.kind === 'recent'), + aheadBehind: options?.aheadBehind ?? repoState?.aheadBehind ?? null, + changedFilesCount: + options?.changedFilesCount ?? repoState?.changedFilesCount ?? 0, + branchName: + options?.branchName ?? + repoState?.branchName ?? + (startupWorktreeEntry + ? getBranchNameForWorktree(startupWorktreeEntry) + : null), + defaultBranchName, + isNestedWorktree, + mainWorktreeName, + isVirtualLinkedWorktree, + isPrunableWorktree: startupWorktreeEntry?.isPrunable ?? false, + worktreePath: options?.worktreePath ?? null, + sourceRepository: options?.sourceRepository ?? parentRepository, + } + } + + const appendVirtualWorktreeItems = ( + items: IRepositoryListItem[], + repository: Repository, + sourceRepository: Repository, + emittedVirtualPaths: Set + ) => { + const repoState = localRepositoryStateLookup.get(repository.id) + const allWorktrees = repoState?.allWorktrees ?? [] + const excludedPaths = new Set([ + ...storedRepositoryPaths, + ...emittedVirtualPaths, + normalizePath(repository.path), + ]) + const virtualWorktrees = allWorktrees + .filter( + worktree => + worktree.type === 'linked' && + !excludedPaths.has(normalizePath(worktree.path)) + ) + .sort((x, y) => + caseInsensitiveCompare(Path.basename(x.path), Path.basename(y.path)) + ) + + for (const worktree of virtualWorktrees) { + const virtualRepositoryPath = normalizePath(worktree.path) + const virtualRepository = new Repository( + worktree.path, + getVirtualRepositoryId(virtualRepositoryPath), + sourceRepository.gitHubRepository, + false, + null, + sourceRepository.groupName, + sourceRepository.defaultBranch, + sourceRepository.workflowPreferences, + sourceRepository.customEditorOverride, + sourceRepository.isTutorialRepository, + sourceRepository.overrideLogin + ) + + items.push( + toListItem(virtualRepository, true, { + isVirtualLinkedWorktree: true, + worktreePath: worktree.path, + sourceRepository, + branchName: getBranchNameForWorktree(worktree), + changedFilesCount: 0, + aheadBehind: null, + }) + ) + emittedVirtualPaths.add(virtualRepositoryPath) + } + } + + const sortedRepositories = [...repositories].sort((x, y) => + caseInsensitiveCompare( + getRepositoryListTitle(x, showWorktreesInSidebar), + getRepositoryListTitle(y, showWorktreesInSidebar) + ) + ) + + if (!showWorktreesInSidebar || group.kind === 'recent') { + return sortedRepositories.map(repository => toListItem(repository, false)) + } + + const mainRepos: Repositoryish[] = [] + const orphanLinkedRepos: Repository[] = [] + const linkedReposByParentPath = new Map() + + for (const repository of sortedRepositories) { + if (!(repository instanceof Repository) || !repository.isLinkedWorktree) { + mainRepos.push(repository) + continue + } + + const parentPath = normalizePath(repository.mainWorktreePath) + const linkedRepos = linkedReposByParentPath.get(parentPath) + if (linkedRepos !== undefined) { + linkedRepos.push(repository) + } else { + linkedReposByParentPath.set(parentPath, [repository]) + } + } + + const items: IRepositoryListItem[] = [] + const seenLinkedRepoIds = new Set() + const emittedVirtualPaths = new Set() + + for (const repository of mainRepos) { + items.push(toListItem(repository, false)) + + if (!(repository instanceof Repository)) { + continue + } + + const linkedRepos = linkedReposByParentPath.get( + normalizePath(repository.path) + ) + if (linkedRepos !== undefined) { + for (const linkedRepo of linkedRepos) { + seenLinkedRepoIds.add(linkedRepo.id) + items.push(toListItem(linkedRepo, true)) + } + } + + appendVirtualWorktreeItems( + items, + repository, + repository, + emittedVirtualPaths + ) + } + + for (const repository of sortedRepositories) { + if ( + repository instanceof Repository && + repository.isLinkedWorktree && + !seenLinkedRepoIds.has(repository.id) + ) { + orphanLinkedRepos.push(repository) + } + } + + for (const repository of orphanLinkedRepos) { + items.push(toListItem(repository, false)) + } + + return items +} diff --git a/app/src/ui/repository-settings/git-config.tsx b/app/src/ui/repository-settings/git-config.tsx index 0bfff798edc..585ab0d1440 100644 --- a/app/src/ui/repository-settings/git-config.tsx +++ b/app/src/ui/repository-settings/git-config.tsx @@ -4,7 +4,15 @@ import { Account } from '../../models/account' import { GitConfigUserForm } from '../lib/git-config-user-form' import { Row } from '../lib/row' import { RadioGroup } from '../lib/radio-group' +import { LinkButton } from '../lib/link-button' import { assertNever } from '../../lib/fatal-error' +import { + IConfigValueOrigin, + getOriginFilePath, + formatConfigScope, + formatConfigPath, +} from '../../lib/git/config' +import { showItemInFolder } from '../main-process-proxy' import memoizeOne from 'memoize-one' interface IGitConfigProps { @@ -17,6 +25,10 @@ interface IGitConfigProps { readonly globalEmail: string readonly isLoadingGitConfig: boolean + readonly nameOrigin?: IConfigValueOrigin | null + readonly emailOrigin?: IConfigValueOrigin | null + readonly repositoryPath: string + readonly onGitConfigLocationChanged: (value: GitConfigLocation) => void readonly onNameChanged: (name: string) => void readonly onEmailChanged: (email: string) => void @@ -85,7 +97,73 @@ export class GitConfig extends React.Component { isLoadingGitConfig={this.props.isLoadingGitConfig} />
+ {this.renderConfigOrigin()} ) } + + private onRevealNameConfigFile = () => { + if (this.props.nameOrigin) { + showItemInFolder( + getOriginFilePath(this.props.nameOrigin, this.props.repositoryPath) + ) + } + } + + private onRevealEmailConfigFile = () => { + if (this.props.emailOrigin) { + showItemInFolder( + getOriginFilePath(this.props.emailOrigin, this.props.repositoryPath) + ) + } + } + + private renderOriginEntry( + key: string, + origin: IConfigValueOrigin, + onReveal: () => void + ) { + const repoPath = this.props.repositoryPath + return ( +
+
+ {key} = {origin.value} +
+
+ Scope: {formatConfigScope(origin)} +
+
+ File:{' '} + + {formatConfigPath(origin, repoPath)} + +
+
+ ) + } + + private renderConfigOrigin() { + const { nameOrigin, emailOrigin } = this.props + if (!nameOrigin && !emailOrigin) { + return null + } + + return ( +
+

Resolved effective identity

+ {nameOrigin && + this.renderOriginEntry( + 'user.name', + nameOrigin, + this.onRevealNameConfigFile + )} + {emailOrigin && + this.renderOriginEntry( + 'user.email', + emailOrigin, + this.onRevealEmailConfigFile + )} +
+ ) + } } diff --git a/app/src/ui/repository-settings/repository-settings.tsx b/app/src/ui/repository-settings/repository-settings.tsx index 18277b2ce4b..bc1f654fd3f 100644 --- a/app/src/ui/repository-settings/repository-settings.tsx +++ b/app/src/ui/repository-settings/repository-settings.tsx @@ -21,8 +21,10 @@ import { GitConfigLocation, GitConfig } from './git-config' import { getConfigValue, getGlobalConfigValue, + getConfigValueWithOrigin, removeConfigValue, setConfigValue, + IConfigValueOrigin, } from '../../lib/git/config' import { gitAuthorNameIsValid, @@ -72,6 +74,8 @@ interface IRepositorySettingsState { readonly errors?: ReadonlyArray readonly forkContributionTarget: ForkContributionTarget readonly isLoadingGitConfig: boolean + readonly nameOrigin: IConfigValueOrigin | null + readonly emailOrigin: IConfigValueOrigin | null readonly availableEditors: ReadonlyArray readonly useDefaultEditor: boolean readonly selectedExternalEditor: string | null @@ -106,6 +110,8 @@ export class RepositorySettings extends React.Component< initialCommitterName: null, initialCommitterEmail: null, isLoadingGitConfig: true, + nameOrigin: null, + emailOrigin: null, availableEditors: [], useDefaultEditor: !props.repository.customEditorOverride, selectedExternalEditor: @@ -163,6 +169,17 @@ export class RepositorySettings extends React.Component< committerEmail = localCommitterEmail ?? '' } + let nameOrigin: IConfigValueOrigin | null = null + let emailOrigin: IConfigValueOrigin | null = null + try { + ;[nameOrigin, emailOrigin] = await Promise.all([ + getConfigValueWithOrigin(this.props.repository, 'user.name'), + getConfigValueWithOrigin(this.props.repository, 'user.email'), + ]) + } catch (e) { + log.warn('Failed to get config value origins', e) + } + this.setState({ gitConfigLocation, committerName, @@ -174,6 +191,8 @@ export class RepositorySettings extends React.Component< initialCommitterName: localCommitterName, initialCommitterEmail: localCommitterEmail, isLoadingGitConfig: false, + nameOrigin, + emailOrigin, }) } @@ -307,6 +326,9 @@ export class RepositorySettings extends React.Component< onNameChanged={this.onCommitterNameChanged} onEmailChanged={this.onCommitterEmailChanged} isLoadingGitConfig={this.state.isLoadingGitConfig} + nameOrigin={this.state.nameOrigin} + emailOrigin={this.state.emailOrigin} + repositoryPath={this.props.repository.path} /> ) } diff --git a/app/src/ui/repository.tsx b/app/src/ui/repository.tsx index 8c7906b2c95..9fd3392e424 100644 --- a/app/src/ui/repository.tsx +++ b/app/src/ui/repository.tsx @@ -64,6 +64,7 @@ interface IRepositoryViewProps { readonly focusCommitMessage: boolean readonly commitSpellcheckEnabled: boolean readonly showCommitLengthWarning: boolean + readonly showCommitAuthorInfo: boolean readonly accounts: ReadonlyArray readonly shouldShowGenerateCommitMessageCallOut: boolean @@ -347,6 +348,8 @@ export class RepositoryView extends React.Component< aheadBehind={this.props.state.aheadBehind} branch={branchName} commitAuthor={this.props.state.commitAuthor} + commitAuthorNameOrigin={this.props.state.commitAuthorNameOrigin} + commitAuthorEmailOrigin={this.props.state.commitAuthorEmailOrigin} emoji={this.props.emoji} mostRecentLocalCommit={mostRecentLocalCommit} issuesStore={this.props.issuesStore} @@ -387,6 +390,7 @@ export class RepositoryView extends React.Component< } commitSpellcheckEnabled={this.props.commitSpellcheckEnabled} showCommitLengthWarning={this.props.showCommitLengthWarning} + showCommitAuthorInfo={this.props.showCommitAuthorInfo} showChangesFilter={this.props.showChangesFilter} hasCommitHooks={this.props.hasCommitHooks} skipCommitHooks={this.props.skipCommitHooks} diff --git a/app/src/ui/toolbar/worktree-dropdown.tsx b/app/src/ui/toolbar/worktree-dropdown.tsx index 7501c2bf697..7fe69309223 100644 --- a/app/src/ui/toolbar/worktree-dropdown.tsx +++ b/app/src/ui/toolbar/worktree-dropdown.tsx @@ -18,6 +18,7 @@ import { PopupType } from '../../models/popup' import { Resizable } from '../resizable' import { enableResizingToolbarButtons } from '../../lib/feature-flag' import { normalizePath } from '../../lib/helpers/path' +import { setPreferredWorktreePath } from '../../lib/worktree-preferences' interface IWorktreeDropdownProps { readonly dispatcher: Dispatcher @@ -54,6 +55,13 @@ export class WorktreeDropdown extends React.Component< dispatcher.closeFoldout(FoldoutType.Worktree) + const { allWorktrees } = this.props.repositoryState.worktreesState + const mainWorktree = allWorktrees.find(wt => wt.type === 'main') + + if (mainWorktree) { + setPreferredWorktreePath(mainWorktree.path, worktree.path) + } + const existingRepo = repositories.find( r => r instanceof Repository && normalizePath(r.path) === worktreePath ) diff --git a/app/src/ui/worktrees/delete-worktree-dialog.tsx b/app/src/ui/worktrees/delete-worktree-dialog.tsx index 2080e3326c5..933886a2a21 100644 --- a/app/src/ui/worktrees/delete-worktree-dialog.tsx +++ b/app/src/ui/worktrees/delete-worktree-dialog.tsx @@ -8,10 +8,16 @@ import { Ref } from '../lib/ref' import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' import { removeWorktree, getMainWorktreePath } from '../../lib/git/worktree' import { normalizePath } from '../../lib/helpers/path' +import { + getPreferredWorktreePath, + clearPreferredWorktreePath, +} from '../../lib/worktree-preferences' interface IDeleteWorktreeDialogProps { readonly repository: Repository readonly worktreePath: string + readonly storedRepositoryToRemove?: Repository + readonly isDeletingCurrentWorktree?: boolean readonly dispatcher: Dispatcher readonly onDismissed: () => void } @@ -62,20 +68,27 @@ export class DeleteWorktreeDialog extends React.Component< private onDeleteWorktree = async () => { this.setState({ isDeleting: true }) - const { repository, worktreePath, dispatcher } = this.props - const isDeletingCurrentWorktree = - normalizePath(repository.path) === normalizePath(worktreePath) + const { + repository, + worktreePath, + dispatcher, + storedRepositoryToRemove, + isDeletingCurrentWorktree = false, + } = this.props + + const mainPathForCleanup = await getMainWorktreePath(repository) try { if (isDeletingCurrentWorktree) { // When deleting the currently selected worktree, we must switch away // first. Otherwise git runs from the directory being deleted and the // app is left pointing at a non-existent path. - const mainPath = await getMainWorktreePath(repository) - if (mainPath === null) { + if (mainPathForCleanup === null) { throw new Error('Could not find main worktree') } + const mainPath = mainPathForCleanup + const addedRepos = await dispatcher.addRepositories( [mainPath], repository.login @@ -87,16 +100,27 @@ export class DeleteWorktreeDialog extends React.Component< const mainRepo = addedRepos[0] await dispatcher.selectRepository(mainRepo) await removeWorktree(mainRepo, worktreePath) - await dispatcher.removeRepository(repository, false) } else { await removeWorktree(repository, worktreePath) } + + if (storedRepositoryToRemove !== undefined) { + await dispatcher.removeRepository(storedRepositoryToRemove, false) + } else if (!isDeletingCurrentWorktree) { + await dispatcher.refreshRepository(repository) + } } catch (e) { dispatcher.postError(e) this.setState({ isDeleting: false }) return } + const resolvedMainPath = mainPathForCleanup ?? repository.path + const preferred = getPreferredWorktreePath(resolvedMainPath) + if (preferred && normalizePath(preferred) === normalizePath(worktreePath)) { + clearPreferredWorktreePath(resolvedMainPath) + } + this.props.onDismissed() } } diff --git a/app/styles/ui/_preferences.scss b/app/styles/ui/_preferences.scss index 3b35ebfdd44..45ba1ba4278 100644 --- a/app/styles/ui/_preferences.scss +++ b/app/styles/ui/_preferences.scss @@ -13,14 +13,6 @@ .hooks-warning { margin-bottom: var(--spacing); } - - .git-hooks-support-description, - .git-hooks-env-description, - .git-hooks-cache-description { - margin-top: var(--spacing); - font-size: var(--font-size-sm); - color: var(--text-secondary-color); - } } .preferences-container { @@ -47,10 +39,6 @@ margin-bottom: var(--spacing-half); } - .git-settings-description { - margin-top: var(--spacing); - } - .setting-hint-warning { margin-top: var(--spacing); } @@ -146,7 +134,7 @@ } .git-settings-description { - margin-top: var(--spacing-double); + margin-top: var(--spacing); font-size: var(--font-size-sm); color: var(--text-secondary-color); } diff --git a/app/styles/ui/_repository-list.scss b/app/styles/ui/_repository-list.scss index aa7dcf31ab1..10d66848223 100644 --- a/app/styles/ui/_repository-list.scss +++ b/app/styles/ui/_repository-list.scss @@ -34,6 +34,10 @@ // name and truncate accordingly width: 100%; + &.nested-worktree { + padding-left: calc(var(--spacing) + 20px); + } + .icon-for-repository { // Some room between the icon and repository name margin-right: var(--spacing-half); @@ -88,6 +92,13 @@ vertical-align: text-bottom; } } + + .worktrees-loading { + color: var(--text-secondary-color); + font-size: var(--font-size-sm); + margin-left: var(--spacing); + flex-shrink: 0; + } } .filter-list-group-header { @@ -216,6 +227,19 @@ } } + .prunable-indicator-wrapper { + display: flex; + min-width: 12px; + justify-content: center; + align-items: center; + margin-left: var(--spacing-half); + + .octicon { + color: var(--toolbar-dropdown-text-warning-color); + width: auto; + } + } + .ahead-behind { height: 16px; background: var(--list-item-badge-background-color); diff --git a/app/styles/ui/changes/_commit-message.scss b/app/styles/ui/changes/_commit-message.scss index c0edd4d2930..ec64db980c7 100644 --- a/app/styles/ui/changes/_commit-message.scss +++ b/app/styles/ui/changes/_commit-message.scss @@ -80,6 +80,44 @@ } } + .commit-author-identity { + display: flex; + flex-direction: row; + align-items: center; + gap: var(--spacing-half); + margin-bottom: var(--spacing-half); + + &.warning { + border-left: 2px solid var(--input-icon-warning-color); + padding-left: var(--spacing-half); + } + + .commit-author-info { + display: flex; + flex-direction: column; + min-width: 0; + line-height: 1.3; + + .commit-author-name { + font-size: var(--font-size-sm); + font-weight: 600; + @include ellipsis; + cursor: default; + } + + .commit-author-email { + font-size: var(--font-size-xs); + color: var(--text-secondary-color); + @include ellipsis; + cursor: default; + + &.warning { + color: var(--input-warning-text-color); + } + } + } + } + .summary { position: relative; display: flex; diff --git a/app/styles/ui/dialogs/_repository-settings.scss b/app/styles/ui/dialogs/_repository-settings.scss index b1552ecf49c..c2d9eb06ee8 100644 --- a/app/styles/ui/dialogs/_repository-settings.scss +++ b/app/styles/ui/dialogs/_repository-settings.scss @@ -57,4 +57,26 @@ padding-left: 0; padding-right: 0; } + + .config-origin-hint { + margin-top: var(--spacing); + + .config-origin-card { + margin-top: var(--spacing-half); + padding: var(--spacing-half) var(--spacing); + background-color: var(--box-alt-background-color); + border-radius: var(--border-radius); + + .config-origin-key { + font-family: var(--font-family-monospace); + font-size: var(--font-size-sm); + font-weight: var(--font-weight-semibold); + } + + .config-origin-detail { + font-size: var(--font-size-xs); + word-break: break-all; + } + } + } } diff --git a/app/styles/ui/window/_tooltips.scss b/app/styles/ui/window/_tooltips.scss index 3b8e5bc664f..110c0f97d1a 100644 --- a/app/styles/ui/window/_tooltips.scss +++ b/app/styles/ui/window/_tooltips.scss @@ -212,6 +212,41 @@ body > .tooltip, } } + &.config-origin { + .config-origin-tooltip { + display: grid; + grid-template-columns: auto 1fr; + column-gap: var(--spacing-half); + + .config-origin-tooltip-label { + font-weight: bold; + text-align: right; + } + + .config-origin-tooltip-warning-icon { + color: var(--input-icon-warning-color); + grid-row: span 3; + display: flex; + align-items: center; + justify-content: center; + } + + .config-origin-tooltip-warning { + color: var(--input-icon-warning-color); + font-size: var(--font-size-sm); + } + + .config-origin-tooltip-spacer { + // Empty first-column cell in the warning grid rows + } + + .config-origin-tooltip-hint { + font-size: var(--font-size-xs); + color: var(--input-icon-warning-color); + } + } + } + &.window-controls-tooltip { background-color: var(--toolbar-tooltip-background-color); box-shadow: 0 8px 24px var(--toolbar-tooltip-shadow-color); diff --git a/app/test/unit/repositories-list-grouping-test.ts b/app/test/unit/repositories-list-grouping-test.ts index 2de41d25bed..90c6087e31d 100644 --- a/app/test/unit/repositories-list-grouping-test.ts +++ b/app/test/unit/repositories-list-grouping-test.ts @@ -1,9 +1,13 @@ import { describe, it } from 'node:test' import assert from 'node:assert' +import * as os from 'node:os' +import * as path from 'node:path' +import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises' import { groupRepositories } from '../../src/ui/repositories-list/group-repositories' import { Repository, ILocalRepositoryState } from '../../src/models/repository' import { CloningRepository } from '../../src/models/cloning-repository' import { gitHubRepoFixture } from '../helpers/github-repo-builder' +import { WorktreeEntry } from '../../src/models/worktree' describe('repository list grouping', () => { const repositories: Array = [ @@ -153,4 +157,325 @@ describe('repository list grouping', () => { assert.equal(grouped[2].items[1].text[0], 'enterprise-repo') assert(grouped[2].items[1].needsDisambiguation) }) + + it('nests linked worktrees under their main repository in non-recent groups', async () => { + const tempRoot = await mkdtemp( + path.join(os.tmpdir(), 'github-desktop-plus-worktree-grouping-') + ) + try { + const mainRepoPath = path.join(tempRoot, 'repo') + const linkedRepoPath = path.join(tempRoot, 'repo-feature-worktree') + + await mkdir(path.join(mainRepoPath, '.git'), { recursive: true }) + await mkdir(path.join(mainRepoPath, '.git', 'worktrees', 'fix-node'), { + recursive: true, + }) + await mkdir(linkedRepoPath, { recursive: true }) + await writeFile( + path.join(linkedRepoPath, '.git'), + 'gitdir: ../repo/.git/worktrees/fix-node\n' + ) + await writeFile( + path.join(mainRepoPath, '.git', 'worktrees', 'fix-node', 'commondir'), + '../..\n' + ) + + const mainRepo = new Repository( + mainRepoPath, + 1, + gitHubRepoFixture({ owner: 'example', name: 'repo' }), + false + ) + const linkedRepo = new Repository( + linkedRepoPath, + 2, + gitHubRepoFixture({ owner: 'example', name: 'repo' }), + false + ) + + const grouped = groupRepositories([linkedRepo, mainRepo], cache, [], { + showWorktreesInSidebar: true, + }) + + assert.equal(grouped.length, 1) + assert.equal(grouped[0].items.length, 2) + assert.equal(grouped[0].items[0].repository.path, mainRepoPath) + assert.equal(grouped[0].items[0].isNestedWorktree, false) + assert.equal(grouped[0].items[1].repository.path, linkedRepoPath) + assert.equal(grouped[0].items[1].isNestedWorktree, true) + assert.equal(grouped[0].items[1].mainWorktreeName, 'repo') + } finally { + await rm(tempRoot, { recursive: true, force: true }) + } + }) + + it('shows unstored linked worktrees under their main repository', async () => { + const tempRoot = await mkdtemp( + path.join(os.tmpdir(), 'github-desktop-plus-worktree-virtual-') + ) + try { + const mainRepoPath = path.join(tempRoot, 'repo') + const unstoredWorktreePath = path.join(tempRoot, 'repo-feature-a') + const mainRepo = new Repository( + mainRepoPath, + 10, + gitHubRepoFixture({ owner: 'example', name: 'repo' }), + false + ) + + const allWorktrees: ReadonlyArray = [ + { + path: mainRepoPath, + head: 'a', + branch: 'refs/heads/main', + isDetached: false, + type: 'main', + isLocked: false, + isPrunable: false, + }, + { + path: unstoredWorktreePath, + head: 'b', + branch: 'refs/heads/feature/a', + isDetached: false, + type: 'linked', + isLocked: false, + isPrunable: false, + }, + ] + + cache.set(mainRepo.id, { + aheadBehind: null, + changedFilesCount: 0, + branchName: 'main', + defaultBranchName: 'main', + allWorktrees, + }) + + const grouped = groupRepositories([mainRepo], cache, [], { + showWorktreesInSidebar: true, + }) + + assert.equal(grouped.length, 1) + assert.equal(grouped[0].items.length, 2) + assert.equal(grouped[0].items[0].repository.path, mainRepoPath) + assert.equal(grouped[0].items[1].worktreePath, unstoredWorktreePath) + assert.equal(grouped[0].items[1].isVirtualLinkedWorktree, true) + assert.equal(grouped[0].items[1].text[0], 'repo-feature-a') + assert.equal(grouped[0].items[1].branchName, 'feature/a') + } finally { + await rm(tempRoot, { recursive: true, force: true }) + } + }) + + it('marks prunable linked worktrees in the sidebar item model', () => { + const mainRepo = new Repository( + '/tmp/repo', + 1, + gitHubRepoFixture({ owner: 'example', name: 'repo' }), + false + ) + + const cache = new Map() + cache.set(mainRepo.id, { + changedFilesCount: 0, + aheadBehind: null, + branchName: 'main', + defaultBranchName: 'main', + allWorktrees: [ + { + path: '/tmp/repo', + head: 'abc', + branch: 'refs/heads/main', + isDetached: false, + type: 'main', + isLocked: false, + isPrunable: false, + }, + { + path: '/tmp/repo-stale', + head: 'def', + branch: 'refs/heads/feature/stale', + isDetached: false, + type: 'linked', + isLocked: false, + isPrunable: true, + }, + ], + }) + + const grouped = groupRepositories([mainRepo], cache, [], { + showWorktreesInSidebar: true, + }) + + assert.equal(grouped.length, 1) + assert.equal(grouped[0].items.length, 2) + assert.equal(grouped[0].items[1].isPrunableWorktree, true) + }) + + it('uses parent preloaded worktree data for stored linked worktree branch names', async () => { + const tempRoot = await mkdtemp( + path.join(os.tmpdir(), 'github-desktop-plus-worktree-branch-fallback-') + ) + try { + const mainRepoPath = path.join(tempRoot, 'repo') + const linkedRepoPath = path.join(tempRoot, 'repo-feature-a') + + await mkdir(path.join(mainRepoPath, '.git'), { recursive: true }) + await mkdir(path.join(mainRepoPath, '.git', 'worktrees', 'feature-a'), { + recursive: true, + }) + await mkdir(linkedRepoPath, { recursive: true }) + await writeFile( + path.join(linkedRepoPath, '.git'), + 'gitdir: ../repo/.git/worktrees/feature-a\n' + ) + await writeFile( + path.join(mainRepoPath, '.git', 'worktrees', 'feature-a', 'commondir'), + '../..\n' + ) + + const mainRepo = new Repository( + mainRepoPath, + 12, + gitHubRepoFixture({ owner: 'example', name: 'repo' }), + false + ) + const linkedRepo = new Repository( + linkedRepoPath, + 13, + gitHubRepoFixture({ owner: 'example', name: 'repo' }), + false + ) + + const allWorktrees: ReadonlyArray = [ + { + path: mainRepoPath, + head: 'a', + branch: 'refs/heads/main', + isDetached: false, + type: 'main', + isLocked: false, + isPrunable: false, + }, + { + path: linkedRepoPath, + head: 'b', + branch: 'refs/heads/feature/a', + isDetached: false, + type: 'linked', + isLocked: false, + isPrunable: false, + }, + ] + + cache.set(mainRepo.id, { + aheadBehind: null, + changedFilesCount: 0, + branchName: 'main', + defaultBranchName: 'main', + allWorktrees, + }) + + const grouped = groupRepositories([linkedRepo, mainRepo], cache, [], { + showWorktreesInSidebar: true, + }) + + assert.equal(grouped.length, 1) + assert.equal(grouped[0].items.length, 2) + assert.equal(grouped[0].items[1].repository.path, linkedRepoPath) + assert.equal(grouped[0].items[1].isNestedWorktree, true) + assert.equal(grouped[0].items[1].branchName, 'feature/a') + } finally { + await rm(tempRoot, { recursive: true, force: true }) + } + }) + + it('does not synthesize linked worktree siblings under orphan linked worktrees', async () => { + const tempRoot = await mkdtemp( + path.join(os.tmpdir(), 'github-desktop-plus-worktree-orphan-leaf-') + ) + try { + const mainRepoPath = path.join(tempRoot, 'repo') + const linkedRepoPath = path.join(tempRoot, 'repo-feature-a') + const secondLinkedRepoPath = path.join(tempRoot, 'repo-feature-b') + + await mkdir(path.join(mainRepoPath, '.git'), { recursive: true }) + await mkdir(path.join(mainRepoPath, '.git', 'worktrees', 'feature-a'), { + recursive: true, + }) + await mkdir(path.join(mainRepoPath, '.git', 'worktrees', 'feature-b'), { + recursive: true, + }) + await mkdir(linkedRepoPath, { recursive: true }) + await writeFile( + path.join(linkedRepoPath, '.git'), + 'gitdir: ../repo/.git/worktrees/feature-a\n' + ) + await writeFile( + path.join(mainRepoPath, '.git', 'worktrees', 'feature-a', 'commondir'), + '../..\n' + ) + await writeFile( + path.join(mainRepoPath, '.git', 'worktrees', 'feature-b', 'commondir'), + '../..\n' + ) + + const linkedRepo = new Repository( + linkedRepoPath, + 20, + gitHubRepoFixture({ owner: 'example', name: 'repo' }), + false, + 'custom alias' + ) + + cache.set(linkedRepo.id, { + aheadBehind: null, + changedFilesCount: 0, + branchName: 'feature/a', + defaultBranchName: 'main', + allWorktrees: [ + { + path: mainRepoPath, + head: 'a', + branch: 'refs/heads/main', + isDetached: false, + type: 'main', + isLocked: false, + isPrunable: false, + }, + { + path: linkedRepoPath, + head: 'b', + branch: 'refs/heads/feature/a', + isDetached: false, + type: 'linked', + isLocked: false, + isPrunable: false, + }, + { + path: secondLinkedRepoPath, + head: 'c', + branch: 'refs/heads/feature/b', + isDetached: false, + type: 'linked', + isLocked: false, + isPrunable: false, + }, + ], + }) + + const grouped = groupRepositories([linkedRepo], cache, [], { + showWorktreesInSidebar: true, + }) + + assert.equal(grouped.length, 1) + assert.equal(grouped[0].items.length, 1) + assert.equal(grouped[0].items[0].repository.path, linkedRepoPath) + assert.equal(grouped[0].items[0].isNestedWorktree, false) + assert.equal(grouped[0].items[0].title, 'custom alias') + } finally { + await rm(tempRoot, { recursive: true, force: true }) + } + }) }) diff --git a/app/test/unit/repositories-store-test.ts b/app/test/unit/repositories-store-test.ts index 5fa00ce517f..a54af384a27 100644 --- a/app/test/unit/repositories-store-test.ts +++ b/app/test/unit/repositories-store-test.ts @@ -3,7 +3,11 @@ import assert from 'node:assert' import { RepositoriesStore } from '../../src/lib/stores/repositories-store' import { TestRepositoriesDatabase } from '../helpers/databases' import { IAPIFullRepository, getDotComAPIEndpoint } from '../../src/lib/api' -import { assertIsRepositoryWithGitHubRepository } from '../../src/models/repository' +import { + assertIsRepositoryWithGitHubRepository, + Repository, +} from '../../src/models/repository' +import { gitHubRepoFixture } from '../helpers/github-repo-builder' describe('RepositoriesStore', () => { let repoDb = new TestRepositoriesDatabase() @@ -97,4 +101,22 @@ describe('RepositoriesStore', () => { ) }) }) + + describe('stash check tracking', () => { + it('ignores transient synthetic repositories', async () => { + const syntheticRepo = new Repository( + '/tmp/repo-feature-a', + -1, + gitHubRepoFixture({ owner: 'example', name: 'repo' }), + false + ) + + assert.equal( + await repositoriesStore.getLastStashCheckDate(syntheticRepo), + null + ) + + await repositoriesStore.updateLastStashCheckDate(syntheticRepo) + }) + }) }) diff --git a/app/test/unit/repository-list-item-context-menu-test.ts b/app/test/unit/repository-list-item-context-menu-test.ts new file mode 100644 index 00000000000..a8dd9c7fe22 --- /dev/null +++ b/app/test/unit/repository-list-item-context-menu-test.ts @@ -0,0 +1,179 @@ +import assert from 'node:assert' +import { describe, it } from 'node:test' + +import { Repository } from '../../src/models/repository' +import { generateRepositoryListContextMenu } from '../../src/ui/repositories-list/repository-list-item-context-menu' +import { gitHubRepoFixture } from '../helpers/github-repo-builder' + +describe('repository list item context menu', () => { + const buildConfig = ( + overrides: Partial< + Parameters[0] + > = {} + ) => { + const repository = + overrides.repository ?? + new Repository( + '/tmp/repo', + 1, + gitHubRepoFixture({ owner: 'example', name: 'repo' }), + false, + 'alias', + 'group' + ) + + return { + repository, + shellLabel: undefined, + externalEditorLabel: undefined, + askForConfirmationOnRemoveRepository: true, + onViewInBrowser: () => {}, + onOpenInNewWindow: () => {}, + onOpenInShell: () => {}, + onShowRepository: () => {}, + onOpenInExternalEditor: () => {}, + onRemoveRepository: () => {}, + onChangeRepositoryAlias: () => {}, + onRemoveRepositoryAlias: () => {}, + onChangeRepositoryGroupName: () => {}, + onRemoveRepositoryGroupName: () => {}, + onCopyRepoPath: () => {}, + ...overrides, + } + } + + it('shows alias and group name actions for normal repository rows', () => { + const items = generateRepositoryListContextMenu(buildConfig()) + const labels = items.flatMap(item => ('label' in item ? [item.label] : [])) + + assert(labels.includes('Change alias') || labels.includes('Change Alias')) + assert(labels.includes('Remove alias') || labels.includes('Remove Alias')) + assert( + labels.includes('Change group name') || + labels.includes('Change Group Name') + ) + assert( + labels.includes('Restore group name') || + labels.includes('Restore Group Name') + ) + assert(labels.includes('Remove…')) + }) + + it('hides alias and group name actions for linked worktree rows and deletes the worktree', () => { + let removedRepository = false + let removedLinkedWorktree = false + + const items = generateRepositoryListContextMenu( + buildConfig({ + isLinkedWorktreeRow: true, + isVirtualLinkedWorktreeRow: true, + onRemoveRepository: () => { + removedRepository = true + }, + onRemoveLinkedWorktree: () => { + removedLinkedWorktree = true + }, + }) + ) + const labels = items.flatMap(item => ('label' in item ? [item.label] : [])) + + assert(!labels.includes('Change alias')) + assert(!labels.includes('Change Alias')) + assert(!labels.includes('Remove alias')) + assert(!labels.includes('Remove Alias')) + assert(!labels.includes('Change group name')) + assert(!labels.includes('Change Group Name')) + assert(!labels.includes('Restore group name')) + assert(!labels.includes('Restore Group Name')) + assert(labels.includes('Delete…')) + + const deleteItem = items.find( + (item): item is { label: string; action: () => void } => + 'label' in item && item.label === 'Delete…' + ) + assert(deleteItem !== undefined) + + deleteItem.action() + + assert.equal(removedLinkedWorktree, true) + assert.equal(removedRepository, false) + }) + + it('keeps alias and group name actions for saved linked worktree rows', () => { + const items = generateRepositoryListContextMenu( + buildConfig({ + isLinkedWorktreeRow: true, + isVirtualLinkedWorktreeRow: false, + onRemoveLinkedWorktree: () => {}, + }) + ) + const labels = items.flatMap(item => ('label' in item ? [item.label] : [])) + + assert(labels.includes('Change alias') || labels.includes('Change Alias')) + assert(labels.includes('Remove alias') || labels.includes('Remove Alias')) + assert(!labels.includes('Change group name')) + assert(!labels.includes('Change Group Name')) + assert(!labels.includes('Restore group name')) + assert(!labels.includes('Restore Group Name')) + assert(labels.includes('Delete…')) + assert(!labels.includes('Remove…')) + }) + + it('shows a prune action for stale worktree rows and keeps remove semantics', () => { + let prunedStaleWorktrees = false + let removedRepository = false + + const items = generateRepositoryListContextMenu( + buildConfig({ + isPrunableWorktreeRow: true, + onPruneStaleWorktrees: () => { + prunedStaleWorktrees = true + }, + onRemoveRepository: () => { + removedRepository = true + }, + }) + ) + const labels = items.flatMap(item => ('label' in item ? [item.label] : [])) + + assert( + labels.includes('Prune stale worktrees') || + labels.includes('Prune Stale Worktrees') + ) + assert(labels.includes('Remove…')) + assert(!labels.includes('Delete…')) + + const pruneItem = items.find( + (item): item is { label: string; action: () => void } => + 'label' in item && + (item.label === 'Prune stale worktrees' || + item.label === 'Prune Stale Worktrees') + ) + assert(pruneItem !== undefined) + + pruneItem.action() + + assert.equal(prunedStaleWorktrees, true) + assert.equal(removedRepository, false) + }) + + it('shows only prune for stale virtual worktree rows', () => { + const items = generateRepositoryListContextMenu( + buildConfig({ + isLinkedWorktreeRow: true, + isVirtualLinkedWorktreeRow: true, + isPrunableWorktreeRow: true, + onPruneStaleWorktrees: () => {}, + }) + ) + const labels = items.flatMap(item => ('label' in item ? [item.label] : [])) + + assert.equal('type' in items[0] && items[0].type === 'separator', false) + assert( + labels.includes('Prune stale worktrees') || + labels.includes('Prune Stale Worktrees') + ) + assert(!labels.includes('Delete…')) + assert(!labels.includes('Remove…')) + }) +}) diff --git a/changelog.json b/changelog.json index dbb73ec9306..f745bdddd64 100644 --- a/changelog.json +++ b/changelog.json @@ -1,5 +1,21 @@ { "releases": { + "3.5.7": [ + "[Added] Rename branch dialog now validates branch names against repository rulesets - #21822", + "[Added] Allow empty commits via the commit options menu - #21771", + "[Added] Add option to include a Signed-off-by trailer on commits for Developer Certificate of Origin workflows - #21741", + "[Fixed] Discard Changes now completes even when the GitHub Desktop window is not focused - #21739", + "[Fixed] Allow origin fetch on empty repo - #4574 #4575. Thanks @jackfreem!", + "[Fixed] Fix window background color when resizing - #21548. Thanks @BaldrianSector!", + "[Fixed] Ignore Tab in terminal and keep textarea enabled - #21696", + "[Fixed] Clear commit message after a successful commit when switching to the History tab while the commit is in progress - #21721", + "[Improved] Liquid Glass icons for macOS Tahoe - #21010. Thanks @fabe and @caiofbpa!", + "[Improved] Update default tab size from 8 to 4 - #21705. Thanks @advaith1!", + "[Improved] Recognize Copilot CLI bot in commit attributions - #21727" + ], + "3.5.7-beta3": [ + "[Added] Rename branch dialog now validates branch names against repository rulesets - #21822" + ], "3.5.7-beta2": [ "[Added] Allow empty commits via the commit options menu - #21771" ], diff --git a/docs/process/first-responder.md b/docs/process/first-responder.md deleted file mode 100644 index d779fa4ced3..00000000000 --- a/docs/process/first-responder.md +++ /dev/null @@ -1,26 +0,0 @@ -# First Responder Rotation - -We have a first responder rotation, aka FRR. The goals of the rotation are: - -1. Ensure community issues and PRs are answered in a timely manner. -1. Ensure support load is shared across the whole team. -1. Free up the rest of the team to focus on milestone work. -1. Give everyone a regular break from milestone work. - -Each rotation is a week long. While first responder your primary duties are: - -1. Triage issues. - * The current first responder is not responsible for following up on issues still open from previous first responders. However, they should highlight to the team the issues that have been left unanswered for at least 5 days - from previous responders to increase visibility and potentially point out which are highest priority. - * At mention @desktop/support and add `support` label if the issue feels applicable to only the user reporting it, and isn't something more broadly relevant. - * Ensure issues are labeled accurately. - * Review issues labeled [`reviewer-needs-to-reproduce`](https://github.com/desktop/desktop/issues?utf8=%E2%9C%93&q=is%3Aopen+is%3Aissue+sort%3Aupdated-asc+label%3Areviewer-needs-to-reproduce) and close any that have gone 2 weeks with no new activity after the last question by a reviewer. - * Review issues labeled [`more-information-needed`](https://github.com/desktop/desktop/issues?q=is%3Aopen+is%3Aissue+label%3Amore-information-needed+sort%3Aupdated-asc) and close any that have gone 7 days without an issue template being filled out. Otherwise, the `no-response` bot will close it after 2 weeks. - * See [issue-triage.md](issue-triage.md) for more information on our issue triage process. -1. Check community pull requests and label ones that are `ready-for-review`. - -Once those things are done, you should feel free to spend your time scratching your own itches on the project. Really wanna refactor that one monstrous component? Go for it! Wanna fix that one bug that drives you nuts? Do it! Wanna upgrade all of our dependencies? You're an awesome masochist! - -That said, tasks which need design work generally *aren't* well-suited to this. It would pull our fantastic designers away from milestone work and it would be hard to get done in a week's time. - -If you're at a loss for ideas or wonder if something is an appropriate first responder task, ask the rest of the team! Or poke through the [`tech-debt`](https://github.com/desktop/desktop/labels/tech-debt) label for some inspiration. diff --git a/docs/process/issue-triage.md b/docs/process/issue-triage.md index 0ad98644490..d8d4e34198f 100644 --- a/docs/process/issue-triage.md +++ b/docs/process/issue-triage.md @@ -1,161 +1,56 @@ # Issue Triage -> Triage (/ˈtriːɑːʒ/ or /triːˈɑːʒ/) is the process of determining the priority -> of patients' treatments based on the severity of their condition. This -> rations patient treatment efficiently when resources are insufficient for all -> to be treated immediately. -> -> *From Wikipedia* +This document describes how we triage issues in [desktop/desktop](https://github.com/desktop/desktop). -The above describes medical triage but it is clear that it also applies to our -situation. Triage is a process of sifting through all the things that we could -work on to select the few things that we will work on. In order to maximize the -impact we have for the people that use GitHub Desktop, things that will get top -priority are items that are well-described, clearly presented and have obvious -benefit. +## Quick Guide -Additionally, we want to encourage helpful feedback and meaningful -participation. In order to do this, we will have to be clear about what we need -from people so that we can deliver what they need. This also means that we will -have to be very clear and decisive when we are not getting the information or -cooperation we need so that we can move on. Just like in an emergency room, if -it is a choice between spending several hours to have a 10% chance of saving -one person or spending several hours definitely saving multiple people, the -choice is clear. +Pick an issue from the First Responder triage queue. -## Goals +**Your goal:** Do what is needed to remove the `needs-triage` label. -* Communicate clearly and effectively - * What the maintainers will work on - * What pull requests will be reviewed for acceptance - * What pull requests *will not* be reviewed for acceptance -* Outline exactly what is expected for an issue to meet the "triage bar" so - that issues that don't meet the bar can be closed -* Reduce the amount of time and back-and-forth needed to take an issue from - being first-opened to `triaged` or closed -* Accept input from the community that helps us deliver meaningful results to - GitHub Desktop and its users +1. **Can we close it?** + - Duplicate → Comment and close as duplicate, linking the original + - Spam → Add `invalid` or `suspected-spam` (auto-closes) + - Abuse → Add `invalid`, remove content, report, block + - Off-topic → Add `off-topic` (auto-closes with comment) -## The Issues List Is Our Backlog +2. **Is it a bug?** + - Reproducible → Add `bug` and a priority label (`priority-1`, `priority-2`, or `priority-3`) + - Not reproducible → Add `unable-to-reproduce` (auto-requests info, 14-day timer) -The GitHub Desktop issues list is what the maintainers team uses to guide our -work. In order for our work to be focused and efficient, our issues list must -be clean and well-organized. Accepting input from the community is a -significant benefit *when it does not distract us from making things better*. +3. **Is it an enhancement?** + - Clear value → Add `enhancement` (auto-posts backlog comment) + - Unclear → Comment for clarification and add `more-info-needed` (14-day timer) -* Untriaged issues are tasks that are being evaluated to determine if they meet - the triage bar -* Open triaged issues are tasks that the maintainers have agreed to work on -* Closed issues are things that either didn't meet the triage bar or are tasks - that the maintainers will not be taking on +The `needs-triage` label is automatically removed when end-state labels (`enhancement`, `bug`, `ready-for-review`) are applied or the issue is closed. -## The Triage Bar +## Priority Levels -In order to be considered triaged an issue **must** contain or be edited to -contain in the body of the issue: +| Priority | Description | +|----------|-------------| +| `priority-1` | Affects many users, prevents core functions. **Escalate in Slack; may require a hotfix.** | +| `priority-2` | Affects multiple users, does not prevent core functions. | +| `priority-3` | Few users affected, cosmetic. | -* The build number associated with the given issue -* The operating system and OS version number that the problem was reproduced on -* Specific steps to reproduce the problem or desired behavior -* If the steps to reproduce the problem do not reproduce it 100% of the time, - an estimate of how often it reproduces with the given steps and configuration -* **One** and only one issue -* Any other information that is required to reproduce the problem (sample Git - repository, specific OS configuration, etc) +## Automated Workflows -### The Body of the Issue +| Label | Automation | +|-------|------------| +| `needs-triage` | Auto-added on open; removed when classified or closed | +| `more-info-needed` | Auto-closes after 14 days without response | +| `unable-to-reproduce` | Auto-adds `more-info-needed` + posts comment | +| `enhancement` | Auto-posts backlog comment | +| `invalid` | Auto-closes immediately | +| `suspected-spam` | Auto-closes immediately | +| `off-topic` | Auto-posts explanation comment + closes | +| `no-help-wanted-issue` | PRs only: Auto-posts explanation comment + closes | +| `ready-for-review` | Auto-removes `needs-triage` + posts acknowledging comment | -You'll notice above that the body of the issue gets special mention. The body -of the issue is the description of the task to be done. A maintainer should -only have to read the body of the issue to understand what needs to happen. -They should not have to read the pages of comments to understand what they need -to do in order to address the issue at hand. +## Off-topic, Spam & Abuse -## Process +- **Off-topic issues:** Add `off-topic` → auto-comments and closes. +- **Spam:** Add `invalid` or `suspected-spam` → auto-closes. +- **Spam comments:** Mark as spam via GitHub. +- **Abuse:** Remove content, report, and use your judgment on blocking. + - Blocking a user from the `desktop` org requires admin permissions. -Keep in mind that this is not the 100% complete maintainer's guide to issues. -This is only a triage process. Once everything has been checked, the issue -reproduced and appropriate labels have been applied, the triage process is done -with the issue. There may be additional maintenance that needs to be done on -issues from time to time that isn't and won't be covered here. - -1. Person files a new issue -1. Maintainer checks to ensure they adequately filled out the template. If not, - close with a request to fill out the template. -1. Label the issue as a `bug` if the issue is a regression or behaviour that - needs to be fixed. -1. Label the issue with `support` if the issue is specific to one person's - configuration and isn't more broadly relevant to other users. -1. If the issue has already been fixed, add a comment linking to the original - issue and close the issue. -1. If anything is unclear but the template is adequately filled out, post what - questions you have and label with `more-information-needed`. -1. Maintainer attempts to reproduce the problem - 1. If the problem is not reproducible, label with `needs-reproduction` and - ask the author of the issue for clarification on the repro steps. -1. Label the issue as an `enhancement` if the issue mentions new behaviour - or functionality that the app should have. - -# Labels - -## More Information Needed - -If a reviewer cannot understand or reproduce the issue with the information provided, they should add a comment indicating what is not clear and add the label `more-information-needed`. - -Although we use a bot, the first responder should also do a manual sweep of issues that are open and labeled `more-information-needed` at least once a week. -* If a `more-information-needed` issue is stale for more than 14 days after the last comment by a reviewer, the issue will be automatically closed by the no-response bot. -* If the original poster did not fill out the issue template and has not responded to our request within 7 days, close the issue with the following message `I'm closing the issue due to inactivity but I'm happy to re-open if you can provide more details.` - -## Support - -If an issue reported feels specific to one user's setup and a solution will likely not be relevant to other users of Desktop, the reviewer should add the label `support` -and @-mention @desktop/support so they're able to work with the user to figure out what's causing the problem. - -## Needs Reproduction - -If a problem is consistently not reproducible, we **need** more information -from the person reporting the problem. If it isn't a simple misunderstanding -about the steps to reproduce the problem, then we should label it -`more-information-needed` as well and follow that process. - -## Bugs - -These are problems with the current app that are identified by users. These -should be reviewed to ensure they: - - - specify the build associated with the issue - - have instructions sufficient to reproduce the issue - - have details about the impact and severity of the issue - -We will use the `more-information-needed` and `reproduction-required` labels to -indicate when issues are incomplete. - -Once enough detail has been captured about the issue, and it can be reproduced -by one of the maintainers, these should be prioritized by the team. Severe bugs -or bugs affecting many users would be prioritized above minor or low impact -bugs. - -## Enhancements - -Changes or improvements to existing features of the application are generally -fine, but should have some review process before they are implemented. -Contributors are encouraged to open issues to discuss enhancements so that other -contributors can see and participate in the discussion, and the core team can -remain transparent about the interactions. - -To ensure the quality of the application remains high over time, the core team -may need to work with the user proposing the change to clarify details before -work should proceed: - - - user interface - appropriate use of styles, layout - - user experience - ensure things are consistent, discoverable - - quality - ensure the change does not adversely affect other features - -e.g. GitHub Desktop should support worktrees as a first class feature. - -## Out-of-scope - -We anticipate ideas or suggestions that don't align with how we see the -application evolving, so we may close issues with an explanation of why. - -e.g. GitHub Desktop should support working with Mercurial repositories. diff --git a/docs/process/pull-requests.md b/docs/process/pull-requests.md index 272bbd8b7d4..192be37ddeb 100644 --- a/docs/process/pull-requests.md +++ b/docs/process/pull-requests.md @@ -1,100 +1,41 @@ -# Pull Request Triage +# Pull Request Review Process -This document outlines how the Desktop team handles pull requests, to ensure -a consistent process for contributions from the core team and the community. +This document describes how pull requests are handled in [desktop/desktop](https://github.com/desktop/desktop). -## The Review Process +## Review Team -1. **Contributor** opens a pull request. If the pull request is still in progress - it should be created in draft mode. -1. When a pull request in progress is ready, the **contributor** should mark it - as ready for review. -1. A member of the reviewer team will give it a quick look over and - add the `ready-for-review` label and add it to any relevant release board. -1. A **reviewer** with bandwidth will appear. -1. **Reviewer** assigns the PR to themselves. -1. **Reviewer** leaves line comments with suggestions or questions. -1. When the **reviewer** is done they comment on the PR with an emoji, meme, - pokémon, or words to that effect. -1. The **contributor** responds to feedback, makes changes, etc. -1. When the **contributor** is ready for the **reviewer** to re-review, they - comment on the PR with an emoji, meme, pokémon or words to that effect. -1. Goto 6 until both parties are happy with the PR. -1. The **reviewer** hits the big green merge button and deletes the branch (if - applicable). +Desktop pull requests are reviewed by the `desktop/code-reviewers` team. Reviews are distributed via GitHub's **load-balanced code review assignment**, which considers each member's recent review requests and outstanding reviews. -Merged contributions are first published to the beta channel (we aim to publish -new versions weekly if contributions are available) before then being -published to the production channel (we aim to publish new versions on a monthly -cadence). +## Review Ownership -### When The Review Is Done +The **Assignee** field indicates who owns the review process for a contribution. While others are welcome to add reviews, the assignee is responsible for seeing the PR through to completion. Assignees can request additional reviews from engineers with relevant domain expertise. -We're using GitHub's review tools to co-ordinate feedback, and we like to be -methodical with our reviews, so you'll probably end up with one of two results: +## Internal Pull Requests - - **Approved**: nothing else to do; the contribution is great! :gem: - - **Request Changes**: there are things to address; reviewer provides details :memo: - -Reviews can take a few iterations, especially for large contributions. Don't -be disheartened if you feel it takes time - we just want to ensure each -contribution is high-quality and that any outstanding questions are resolved, -captured or documented for posterity. - -### Assignees - -The reviewers team uses the **Assignee** field to indicate who "owns" the review -process for a contribution. While others can add their reviews to a pull request - -and large features will likely have multiple reviewers - it's up to the assignee -to take charge of the process and see it through to the end. - -If a reviewer is feeling overloaded, or if a review has stalled, the reviewer may -remove themselves from a pull request. This helps others know where they can help -out to share the load. - -### Everyone Reviews - -While everyone has their own domain expertise around the codebase, we encourage -people to share the load of reviews and reviewing areas of the codebase that -aren't as familiar. This spreads knowledge across the team - -### 24 Hours Cooling Off - -After being approved, most contributions will remain in this state for at least -24 hours before merging. The review team does this to ensure everyone on the team, -who are normally spread around the world, has a chance to provide feedback about -the changes. +1. **Contributor** opens a pull request (use draft mode if still in progress). +2. When ready, the contributor marks it ready for review. +3. A reviewer is **auto-assigned** via load-balanced code review assignment. +4. **Reviewer** leaves feedback; contributor responds and iterates. +5. Once approved, follow the [24-hour cooling-off period](#24-hour-cooling-off-period) before merging. ### No Self-Merges Without Review -We encourage a strong review culture, and contributors should not merge their -own PRs unless there are exceptional reasons. - -Examples of exceptional situations: - -- [#2733](https://github.com/desktop/desktop/pull/2733) was pinning a dependency - that affected our continuous integration tests by installing the incorrect - version - -- [#4319](https://github.com/desktop/desktop/pull/4319) was a minor packaging - change introduced unexpectedly on `development` but would affect everyone when they - updated their local repositories. - -These should be called out by the merging person with an explanation for why they are bypassing the review process. +Contributors should not merge their own PRs unless there are exceptional reasons (e.g., urgent CI fixes or packaging hotfixes). These should be called out with an explanation for bypassing the review process. -### Stale Pull Requests +## External Pull Requests -A reviewer will return to an **open and reviewed** pull request if, after 14 -days: +External PRs follow this workflow: - - no response has been received from the contributor, or - - no new commits have been made to the pull request +1. **Initial triage** — PR receives the `external` and `needs-triage` labels. The First Responder performs a quick validity check: + - Spam or AI sludge → Add `invalid` (auto-closes) + - Not linked to a help-wanted issue → Add `no-help-wanted-issue` (auto-closes with comment) + - Tiny fix (e.g., typo) → Review, test, and merge directly + - Valid → Add `ready-for-review` and run CI (auto-removes `needs-triage`, auto-posts acknowledging comment) +2. **Review assignment** — The auto-assigned reviewer does not act until `ready-for-review` is applied. +3. **Requesting changes** — If changes are requested, the `contributor-input-needed` label is auto-added (removes `ready-for-review`). When the contributor responds, `ready-for-review` is re-applied automatically. +4. **Stale PR handling** — If no activity for 7 days, an automated message asks if the contributor is still interested. After another 7 days of inactivity, the PR auto-closes. -This is done to ensure we keep the number of reviews under control. +## 24-Hour Cooling-Off Period -The reviewer should ask if the contributor is still interested in working on -this, and indicate that they are welcome to open a fresh pull request later if -they lack the time currently to continue on. +After approval, most contributions remain in this state for at least 24 hours before merging. This ensures all team members across time zones have a chance to provide feedback. -If it's agreed to put this contribution on hold, or if no feedback is -received after 3 days, the pull request will be closed. diff --git a/yarn.lock b/yarn.lock index 6f951abd5fc..3945dbf5ded 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4281,9 +4281,9 @@ flat@^5.0.2: integrity sha512-b6suED+5/3rTpUBdG1gupIl8MPFCAMA0QXwmljLhvCUKcUvdE4gWky9zpuGCcXHOsz4J9wPGNWq6OKpmIzz3hQ== flatted@^3.2.9: - version "3.2.9" - resolved "https://registry.yarnpkg.com/flatted/-/flatted-3.2.9.tgz#7eb4c67ca1ba34232ca9d2d93e9886e611ad7daf" - integrity sha512-36yxDn5H7OFZQla0/jFJmbIKTdZAQHngCedGxiMmpNfEZM0sdEeT+WczLQrjK6D7o2aiyLYDnkw0R3JK0Qv1RQ== + version "3.4.2" + resolved "https://registry.yarnpkg.com/flatted/-/flatted-3.4.2.tgz#f5c23c107f0f37de8dbdf24f13722b3b98d52726" + integrity sha512-PjDse7RzhcPkIJwy5t7KPWQSZ9cAbzQXcafsetQoD7sOJRQlGikNbx7yZp2OotDnJyrDcbyRq3Ttb18iYOqkxA== flora-colossus@^2.0.0: version "2.0.0" @@ -6854,15 +6854,10 @@ picocolors@^1.0.0, picocolors@^1.0.1: resolved "https://registry.yarnpkg.com/picocolors/-/picocolors-1.0.1.tgz#a8ad579b571952f0e5d25892de5445bcfe25aaa1" integrity sha512-anP1Z8qwhkbmu7MFP5iTt+wQKXgwzf7zTyGlcdzabySa9vd0Xt392U0rVmz9poOaBj0uHJKyyo9/upk0HrEQew== -picomatch@^2.0.4, picomatch@^2.2.1: - version "2.2.2" - resolved "https://registry.yarnpkg.com/picomatch/-/picomatch-2.2.2.tgz#21f333e9b6b8eaff02468f5146ea406d345f4dad" - integrity sha512-q0M/9eZHzmr0AulXyPwNfZjtwZ/RBZlbN3K3CErVrk50T2ASYI7Bye0EvekFY3IP1Nt2DHu0re+V2ZHIpMkuWg== - -picomatch@^2.3.1: - version "2.3.1" - resolved "https://registry.yarnpkg.com/picomatch/-/picomatch-2.3.1.tgz#3ba3833733646d9d3e4995946c1365a67fb07a42" - integrity sha512-JU3teHTNjmE2VCGFzuY8EXzCDVwEqB2a8fsIvwaStHhAWJEeVd1o1QD80CU6+ZdEXXSLbSsuLwJjkCBWqRQUVA== +picomatch@^2.0.4, picomatch@^2.2.1, picomatch@^2.3.1: + version "2.3.2" + resolved "https://registry.yarnpkg.com/picomatch/-/picomatch-2.3.2.tgz#5a942915e26b372dc0f0e6753149a16e6b1c5601" + integrity sha512-V7+vQEJ06Z+c5tSye8S+nHUfI51xoXIXjHQ99cQtKUkQqqO1kO/KCJUfZXuB47h/YBlDhah2H3hdUGXn8ie0oA== pify@^2.0.0: version "2.3.0" @@ -7909,7 +7904,14 @@ string_decoder@^1.1.1: dependencies: safe-buffer "~5.2.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1": + version "6.0.1" + resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" + integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== + dependencies: + ansi-regex "^5.0.1" + +strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==