development/github-desktop
development/github-desktop
1
0
Fork 0
mirror of https://github.com/desktop/desktop synced 2024-10-30 06:03:10 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge branch 'development' into upgrade-to-node-10

Browse source
This commit is contained in:
Brendan Forster 2019-06-25 15:44:36 -03:00 committed by GitHub
commit 5907d1d2e5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
31 changed files with 761 additions and 236 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/package.json
View file

@ -3,7 +3,7 @@
"productName": "GitHub Desktop",
"bundleID": "com.github.GitHubClient",
"companyName": "GitHub, Inc.",
"version": "2.0.4-test2",
"version": "2.1.0-beta1",
"main": "./main.js",
"repository": {
"type": "git",

15
app/src/lib/app-state.ts
View file

@ -256,10 +256,20 @@ export type AppMenuFoldout = {
openedWithAccessKey?: boolean
}
export type BranchFoldout = {
type: FoldoutType.Branch
/**
* A flag to indicate the user clicked the "switch branch" link when they
* saw the prompt about the current branch being protected.
*/
handleProtectedBranchWarning?: boolean
}
export type Foldout =
| { type: FoldoutType.Repository }
| { type: FoldoutType.Branch }
| { type: FoldoutType.AddMenu }
| BranchFoldout
| AppMenuFoldout
export enum RepositorySectionTab {
@ -577,6 +587,9 @@ export interface IChangesState {
* for more information about the differences between the two.
*/
readonly selection: ChangesSelection
/** `true` if the GitHub API reports that the branch is protected */
readonly currentBranchProtected: boolean
}
/**

5
app/src/lib/feature-flag.ts
View file

@ -95,6 +95,11 @@ export function enableBranchProtectionChecks(): boolean {
return true
}
/** Should the app detect Windows Subsystem for Linux as a valid shell? */
export function enableWSLDetection(): boolean {
return enableDevelopmentFeatures()
}
/**
* Should the application warn the user when they are about to commit to a
* protected branch, and encourage them into a flow to move their changes to

19
app/src/lib/git/branch.ts
View file

@ -6,18 +6,6 @@ import { IGitAccount } from '../../models/git-account'
import { envForAuthentication } from './authentication'
import { formatAsLocalRef } from './refs'
export interface IMergedBranch {
/**
* The canonical reference to the merged branch
*/
readonly canonicalRef: string
/**
* The full-length Object ID (SHA) in HEX (32 chars)
*/
readonly sha: string
}
/**
* Create a new branch from the given start point.
*
@ -184,11 +172,12 @@ export async function getBranchesPointedAt(
*
* @param repository The repository in which to search
* @param branchName The to be used as the base branch
* @returns map of branch canonical refs paired to its sha
*/
export async function getMergedBranches(
repository: Repository,
branchName: string
): Promise<ReadonlyArray<IMergedBranch>> {
): Promise<Map<string, string>> {
const canonicalBranchRef = formatAsLocalRef(branchName)
const args = [
@ -203,7 +192,7 @@ export async function getMergedBranches(
// Remove the trailing newline
lines.splice(-1, 1)
const mergedBranches = new Array<IMergedBranch>()
const mergedBranches = new Map<string, string>()
for (const line of lines) {
const [sha, canonicalRef] = line.split('\0')
@ -218,7 +207,7 @@ export async function getMergedBranches(
continue
}
mergedBranches.push({ sha, canonicalRef })
mergedBranches.set(canonicalRef, sha)
}
return mergedBranches

57
app/src/lib/shells/win32.ts
View file

@ -5,6 +5,7 @@ import { pathExists } from 'fs-extra'
import { assertNever } from '../fatal-error'
import { IFoundShell } from './found-shell'
import { enableWSLDetection } from '../feature-flag'
export enum Shell {
Cmd = 'Command Prompt',
@ -13,6 +14,7 @@ export enum Shell {
Hyper = 'Hyper',
GitBash = 'Git Bash',
Cygwin = 'Cygwin',
WSL = 'WSL',
}
export const Default = Shell.Cmd
@ -42,6 +44,10 @@ export function parse(label: string): Shell {
return Shell.Cygwin
}
if (label === Shell.WSL) {
return Shell.WSL
}
return Default
}
@ -95,6 +101,16 @@ export async function getAvailableShells(): Promise<
})
}
if (enableWSLDetection()) {
const wslPath = await findWSL()
if (wslPath != null) {
shells.push({
shell: Shell.WSL,
path: wslPath,
})
}
}
return shells
}
@ -268,6 +284,45 @@ async function findCygwin(): Promise<string | null> {
return null
}
async function findWSL(): Promise<string | null> {
const system32 = Path.join(
process.env.SystemRoot || 'C:\\Windows',
'System32'
)
const wslPath = Path.join(system32, 'wsl.exe')
const wslConfigPath = Path.join(system32, 'wslconfig.exe')
if (!(await pathExists(wslPath))) {
log.debug(`[WSL] wsl.exe does not exist at '${wslPath}'`)
return null
}
if (!(await pathExists(wslConfigPath))) {
log.debug(
`[WSL] found wsl.exe, but wslconfig.exe does not exist at '${wslConfigPath}'`
)
return null
}
const exitCode = new Promise<number>((resolve, reject) => {
const wslDistros = spawn(wslConfigPath, ['/list'])
wslDistros.on('error', reject)
wslDistros.on('exit', resolve)
})
try {
const result = await exitCode
if (result !== 0) {
log.debug(
`[WSL] found wsl.exe and wslconfig.exe, but no distros are installed. Error Code: ${result}`
)
return null
}
return wslPath
} catch (err) {
log.error(`[WSL] unhandled error when invoking 'wsl /list'`, err)
}
return null
}
export function launch(
foundShell: IFoundShell<Shell>,
path: string
@ -312,6 +367,8 @@ export function launch(
cwd: path,
}
)
case Shell.WSL:
return spawn('START', ['wsl'], { shell: true, cwd: path })
case Shell.Cmd:
return spawn('START', ['cmd'], { shell: true, cwd: path })
default:

103
app/src/lib/stores/app-store.ts
View file

@ -184,7 +184,11 @@ import {
hasSeenUsageStatsNote,
} from '../stats'
import { hasShownWelcomeFlow, markWelcomeFlowComplete } from '../welcome'
import { getWindowState, WindowState } from '../window-state'
import {
getWindowState,
WindowState,
windowStateChannelName,
} from '../window-state'
import { TypedBaseStore } from './base-store'
import { AheadBehindUpdater } from './helpers/ahead-behind-updater'
import { MergeResult } from '../../models/merge'
@ -216,6 +220,7 @@ import {
enableGroupRepositoriesByOwner,
enableStashing,
enableBranchProtectionChecks,
enableBranchProtectionWarningFlow,
} from '../feature-flag'
import { Banner, BannerType } from '../../models/banner'
import * as moment from 'moment'
@ -412,9 +417,9 @@ export class AppStore extends TypedBaseStore<IAppState> {
private wireupIpcEventHandlers(window: Electron.BrowserWindow) {
ipcRenderer.on(
'window-state-changed',
(event: Electron.IpcMessageEvent, args: any[]) => {
this.windowState = getWindowState(window)
windowStateChannelName,
(event: Electron.IpcMessageEvent, windowState: WindowState) => {
this.windowState = windowState
this.emitUpdate()
}
)
@ -723,6 +728,35 @@ export class AppStore extends TypedBaseStore<IAppState> {
}
}
private async refreshBranchProtectionState(repository: Repository) {
if (!enableBranchProtectionWarningFlow()) {
return
}
const gitStore = this.gitStoreCache.get(repository)
if (
gitStore.tip.kind === TipState.Valid &&
repository.gitHubRepository !== null
) {
const branchName = findRemoteBranchName(
gitStore.tip,
gitStore.currentRemote,
repository.gitHubRepository
)
if (branchName !== null) {
const currentBranchProtected = await this.repositoriesStore.isBranchProtectedOnRemote(
repository.gitHubRepository,
branchName
)
this.repositoryStateCache.updateChangesState(repository, () => ({
currentBranchProtected,
}))
}
}
}
private clearSelectedCommit(repository: Repository) {
this.repositoryStateCache.updateCommitSelection(repository, () => ({
sha: null,
@ -2567,6 +2601,7 @@ export class AppStore extends TypedBaseStore<IAppState> {
gitStore.updateLastFetched(),
gitStore.loadStashEntries(),
this.refreshAuthor(repository),
this.refreshBranchProtectionState(repository),
refreshSectionPromise,
])
@ -2858,7 +2893,11 @@ export class AppStore extends TypedBaseStore<IAppState> {
return repository
}
const repo = await this._checkoutBranch(repository, branch)
const repo = await this._checkoutBranch(
repository,
branch,
uncommittedChangesStrategy
)
this._closePopup()
return repo
}
@ -3112,15 +3151,45 @@ export class AppStore extends TypedBaseStore<IAppState> {
return repository
}
const branches = enableBranchProtectionChecks()
? await api.fetchProtectedBranches(owner, name)
: new Array<IAPIBranch>()
const endpoint = matchedGitHubRepository.endpoint
return this.repositoriesStore.updateGitHubRepository(
const updatedRepository = await this.repositoriesStore.updateGitHubRepository(
repository,
endpoint,
apiRepo,
apiRepo
)
await this.updateBranchProtectionsFromAPI(repository)
return updatedRepository
}
private async updateBranchProtectionsFromAPI(repository: Repository) {
if (
repository.gitHubRepository === null ||
repository.gitHubRepository.dbID === null
) {
return
}
const { owner, name } = repository.gitHubRepository
const account = getAccountForEndpoint(
this.accounts,
repository.gitHubRepository.endpoint
)
if (account === null) {
return
}
const api = API.fromAccount(account)
const branches = enableBranchProtectionChecks()
? await api.fetchProtectedBranches(owner.login, name)
: new Array<IAPIBranch>()
await this.repositoriesStore.updateBranchProtections(
repository.gitHubRepository,
branches
)
}
@ -3323,6 +3392,10 @@ export class AppStore extends TypedBaseStore<IAppState> {
value: refreshStartProgress,
})
// manually refresh branch protections after the push, to ensure
// any new branch will immediately report as protected
await this.updateBranchProtectionsFromAPI(repository)
await this._refreshRepository(repository)
this.updatePushPullFetchProgress(repository, {
@ -3515,6 +3588,10 @@ export class AppStore extends TypedBaseStore<IAppState> {
await gitStore.reconcileHistory(mergeBase)
}
// manually refresh branch protections after the push, to ensure
// any new branch will immediately report as protected
await this.updateBranchProtectionsFromAPI(repository)
await this._refreshRepository(repository)
this.updatePushPullFetchProgress(repository, {
@ -3813,6 +3890,10 @@ export class AppStore extends TypedBaseStore<IAppState> {
value: fetchWeight,
})
// manually refresh branch protections after the push, to ensure
// any new branch will immediately report as protected
await this.updateBranchProtectionsFromAPI(repository)
await this._refreshRepository(repository)
this.updatePushPullFetchProgress(repository, {

82
app/src/lib/stores/helpers/branch-pruner.ts
View file

@ -6,9 +6,9 @@ import {
getMergedBranches,
getBranchCheckouts,
getSymbolicRef,
IMergedBranch,
formatAsLocalRef,
deleteLocalBranch,
getBranches,
} from '../../git'
import { fatalError } from '../../fatal-error'
import { RepositoryStateCache } from '../repository-state-cache'
@ -96,27 +96,28 @@ export class BranchPruner {
})
}
/** @returns a map of canonical refs to their shas */
private async findBranchesMergedIntoDefaultBranch(
repository: Repository,
defaultBranch: Branch
): Promise<ReadonlyArray<IMergedBranch>> {
): Promise<ReadonlyMap<string, string>> {
const gitStore = this.gitStoreCache.get(repository)
const mergedBranches = await gitStore.performFailableOperation(() =>
getMergedBranches(repository, defaultBranch.name)
)
if (mergedBranches === undefined) {
return []
return new Map<string, string>()
}
const currentBranchCanonicalRef = await getSymbolicRef(repository, 'HEAD')
// remove the current branch
return currentBranchCanonicalRef === null
? mergedBranches
: mergedBranches.filter(
mb => mb.canonicalRef !== currentBranchCanonicalRef
)
if (currentBranchCanonicalRef) {
mergedBranches.delete(currentBranchCanonicalRef)
}
return mergedBranches
}
/**
@ -127,7 +128,8 @@ export class BranchPruner {
private async pruneLocalBranches(
options: PruneRuntimeOptions
): Promise<void> {
if (this.repository.gitHubRepository === null) {
const { gitHubRepository } = this.repository
if (gitHubRepository === null) {
return
}
@ -163,7 +165,7 @@ export class BranchPruner {
// Get list of branches that have been merged
const { branchesState } = this.repositoriesStateCache.get(this.repository)
const { defaultBranch } = branchesState
const { defaultBranch, allBranches } = branchesState
if (defaultBranch === null) {
return
@ -174,7 +176,7 @@ export class BranchPruner {
defaultBranch
)
if (mergedBranches.length === 0) {
if (mergedBranches.size === 0) {
log.info('[BranchPruner] No branches to prune.')
return
}
@ -191,13 +193,27 @@ export class BranchPruner {
[...recentlyCheckedOutBranches.keys()].map(formatAsLocalRef)
)
// Create array of branches that can be pruned
const candidateBranches = mergedBranches.filter(
mb => !ReservedRefs.includes(mb.canonicalRef)
)
// get the locally cached branches of remotes (ie `remotes/origin/master`)
const remoteBranches = (await getBranches(
this.repository,
`refs/remotes/`
)).map(b => formatAsLocalRef(b.name))
const branchesReadyForPruning = candidateBranches.filter(
mb => !recentlyCheckedOutCanonicalRefs.has(mb.canonicalRef)
// create list of branches to be pruned
const branchesReadyForPruning = Array.from(mergedBranches.keys()).filter(
ref => {
if (ReservedRefs.includes(ref)) {
return false
}
if (recentlyCheckedOutCanonicalRefs.has(ref)) {
return false
}
const upstreamRef = getUpstreamRefForLocalBranchRef(ref, allBranches)
if (upstreamRef === undefined) {
return false
}
return !remoteBranches.includes(upstreamRef)
}
)
log.info(
@ -211,12 +227,12 @@ export class BranchPruner {
const gitStore = this.gitStoreCache.get(this.repository)
const branchRefPrefix = `refs/heads/`
for (const branch of branchesReadyForPruning) {
if (!branch.canonicalRef.startsWith(branchRefPrefix)) {
for (const branchCanonicalRef of branchesReadyForPruning) {
if (!branchCanonicalRef.startsWith(branchRefPrefix)) {
continue
}
const branchName = branch.canonicalRef.substr(branchRefPrefix.length)
const branchName = branchCanonicalRef.substr(branchRefPrefix.length)
if (options.deleteBranch) {
const isDeleted = await gitStore.performFailableOperation(() =>
@ -225,7 +241,9 @@ export class BranchPruner {
if (isDeleted) {
log.info(
`[BranchPruner] Pruned branch ${branchName} (was ${branch.sha})`
`[BranchPruner] Pruned branch ${branchName} ((was ${mergedBranches.get(
branchCanonicalRef
)}))`
)
}
} else {
@ -235,3 +253,25 @@ export class BranchPruner {
this.onPruneCompleted(this.repository)
}
}
/**
* @param ref the canonical ref for a local branch
* @param allBranches a list of all branches in the Repository model
* @returns the canonical upstream branch ref or undefined if upstream can't be reliably determined
*/
function getUpstreamRefForLocalBranchRef(
ref: string,
allBranches: ReadonlyArray<Branch>
): string | undefined {
const branch = allBranches.find(b => formatAsLocalRef(b.name) === ref)
// if we can't find a branch model, we can't determine the ref's upstream
if (branch === undefined) {
return undefined
}
const { upstream } = branch
// if there's no upstream in the branch, there's nothing to lookup
if (upstream === null) {
return undefined
}
return formatAsLocalRef(upstream)
}

111
app/src/lib/stores/repositories-store.ts
View file

@ -388,8 +388,7 @@ export class RepositoriesStore extends BaseStore {
public async updateGitHubRepository(
repository: Repository,
endpoint: string,
gitHubRepository: IAPIRepository,
branches: ReadonlyArray<IAPIBranch>
gitHubRepository: IAPIRepository
): Promise<Repository> {
const repoID = repository.id
if (!repoID) {
@ -402,7 +401,6 @@ export class RepositoriesStore extends BaseStore {
'rw',
this.db.repositories,
this.db.gitHubRepositories,
this.db.protectedBranches,
this.db.owners,
async () => {
const localRepo = (await this.db.repositories.get(repoID))!
@ -415,50 +413,6 @@ export class RepositoriesStore extends BaseStore {
gitHubRepositoryID: updatedGitHubRepo.dbID,
})
if (enableBranchProtectionChecks()) {
const repoId = updatedGitHubRepo.dbID!
// This update flow is organized into two stages:
//
// - update the in-memory cache
// - update the underyling database state
//
// This should ensure any stale values are not being used, and avoids
// the need to query the database while the results are in memory.
const prefix = getKeyPrefix(repoId)
for (const key of this.protectionEnabledForBranchCache.keys()) {
// invalidate any cached entries belonging to this repository
if (key.startsWith(prefix)) {
this.protectionEnabledForBranchCache.delete(key)
}
}
const branchRecords = branches.map<IDatabaseProtectedBranch>(b => ({
repoId,
name: b.name,
}))
// update cached values to avoid database lookup
for (const item of branchRecords) {
const key = getKey(repoId, item.name)
this.protectionEnabledForBranchCache.set(key, true)
}
await this.db.protectedBranches
.where('repoId')
.equals(repoId)
.delete()
const protectionsFound = branchRecords.length > 0
this.branchProtectionSettingsFoundCache.set(repoId, protectionsFound)
if (branchRecords.length > 0) {
await this.db.protectedBranches.bulkAdd(branchRecords)
}
}
return updatedGitHubRepo
}
)
@ -473,6 +427,69 @@ export class RepositoriesStore extends BaseStore {
)
}
/** Add or update the branch protections associated with a GitHub repository. */
public async updateBranchProtections(
gitHubRepository: GitHubRepository,
protectedBranches: ReadonlyArray<IAPIBranch>
): Promise<void> {
if (!enableBranchProtectionChecks()) {
return
}
const dbID = gitHubRepository.dbID
if (!dbID) {
return fatalError(
'`updateBranchProtections` can only update a GitHub repository for a repository which has been added to the database.'
)
}
await this.db.transaction('rw', this.db.protectedBranches, async () => {
// This update flow is organized into two stages:
//
// - update the in-memory cache
// - update the underyling database state
//
// This should ensure any stale values are not being used, and avoids
// the need to query the database while the results are in memory.
const prefix = getKeyPrefix(dbID)
for (const key of this.protectionEnabledForBranchCache.keys()) {
// invalidate any cached entries belonging to this repository
if (key.startsWith(prefix)) {
this.protectionEnabledForBranchCache.delete(key)
}
}
const branchRecords = protectedBranches.map<IDatabaseProtectedBranch>(
b => ({
repoId: dbID,
name: b.name,
})
)
// update cached values to avoid database lookup
for (const item of branchRecords) {
const key = getKey(dbID, item.name)
this.protectionEnabledForBranchCache.set(key, true)
}
await this.db.protectedBranches
.where('repoId')
.equals(dbID)
.delete()
const protectionsFound = branchRecords.length > 0
this.branchProtectionSettingsFoundCache.set(dbID, protectionsFound)
if (branchRecords.length > 0) {
await this.db.protectedBranches.bulkAdd(branchRecords)
}
})
this.emitUpdate()
}
/**
* Set's the last time the repository was checked for pruning
*

1
app/src/lib/stores/repository-state-cache.ts
View file

@ -134,6 +134,7 @@ function getInitialRepositoryState(): IRepositoryState {
showCoAuthoredBy: false,
conflictState: null,
stashEntry: null,
currentBranchProtected: false,
},
selectedSection: RepositorySectionTab.Changes,
branchesState: {

7
app/src/lib/window-state.ts
View file

@ -45,7 +45,12 @@ export function registerWindowStateChangedEvents(
window.on('unmaximize', () => sendWindowStateEvent(window, 'normal'))
window.on('restore', () => sendWindowStateEvent(window, 'normal'))
window.on('hide', () => sendWindowStateEvent(window, 'hidden'))
window.on('show', () => sendWindowStateEvent(window, 'normal'))
window.on('show', () => {
// because the app can be maximized before being closed - which will restore it
// maximized on the next launch - this function should inspect the current state
// rather than always assume it is a 'normal' launch
sendWindowStateEvent(window, getWindowState(window))
})
}
/**

7
app/src/models/popup.ts
View file

@ -82,6 +82,13 @@ export type Popup =
| {
type: PopupType.CreateBranch
repository: Repository
/**
* A flag to indicate the user clicked the "switch branch" link when they
* saw the prompt about the current branch being protected.
*/
handleProtectedBranchWarning?: boolean
initialName?: string
}
| { type: PopupType.SignIn }

12
app/src/ui/app.tsx
View file

@ -1363,6 +1363,7 @@ export class App extends React.Component<IAppProps, IAppState> {
onDismissed={this.onPopupDismissed}
dispatcher={this.props.dispatcher}
initialName={popup.initialName || ''}
handleProtectedBranchWarning={popup.handleProtectedBranchWarning}
/>
)
}
@ -2152,8 +2153,14 @@ export class App extends React.Component<IAppProps, IAppState> {
}
const currentFoldout = this.state.currentFoldout
const isOpen =
!!currentFoldout && currentFoldout.type === FoldoutType.Branch
let isOpen = false
let handleProtectedBranchWarning: boolean | undefined
if (currentFoldout !== null && currentFoldout.type === FoldoutType.Branch) {
isOpen = true
handleProtectedBranchWarning = currentFoldout.handleProtectedBranchWarning
}
const repository = selection.repository
const branchesState = selection.state.branchesState
@ -2169,6 +2176,7 @@ export class App extends React.Component<IAppProps, IAppState> {
pullRequests={branchesState.openPullRequests}
currentPullRequest={branchesState.currentPullRequest}
isLoadingPullRequests={branchesState.isLoadingPullRequests}
handleProtectedBranchWarning={handleProtectedBranchWarning}
/>
)
}

5
app/src/ui/branches/branch-list.tsx
View file

@ -19,6 +19,7 @@ import {
BranchGroupIdentifier,
} from './group-branches'
import { NoBranches } from './no-branches'
import { SelectionDirection } from '../lib/list'
const RowHeight = 30
@ -161,9 +162,9 @@ export class BranchList extends React.Component<
this.setState(createState(nextProps))
}
public selectFirstItem(focus: boolean = false) {
public selectNextItem(focus: boolean = false, direction: SelectionDirection) {
if (this.branchFilterList !== null) {
this.branchFilterList.selectFirstItem(focus)
this.branchFilterList.selectNextItem(focus, direction)
}
}

35
app/src/ui/branches/branches-container.tsx
View file

@ -22,6 +22,11 @@ import { IBranchListItem } from './group-branches'
import { renderDefaultBranch } from './branch-renderer'
import { IMatches } from '../../lib/fuzzy-find'
import { startTimer } from '../lib/timing'
import {
UncommittedChangesStrategyKind,
UncommittedChangesStrategy,
askToStash,
} from '../../models/uncommitted-changes-strategy'
interface IBranchesContainerProps {
readonly dispatcher: Dispatcher
@ -38,6 +43,9 @@ interface IBranchesContainerProps {
/** Are we currently loading pull requests? */
readonly isLoadingPullRequests: boolean
/** Was this component launched from the "Protected Branch" warning message? */
readonly handleProtectedBranchWarning?: boolean
}
interface IBranchesContainerState {
@ -234,16 +242,26 @@ export class BranchesContainer extends React.Component<
private onBranchItemClick = (branch: Branch) => {
this.props.dispatcher.closeFoldout(FoldoutType.Branch)
const currentBranch = this.props.currentBranch
const {
currentBranch,
repository,
handleProtectedBranchWarning,
} = this.props
if (currentBranch == null || currentBranch.name !== branch.name) {
const timer = startTimer(
'checkout branch from list',
this.props.repository
)
const timer = startTimer('checkout branch from list', repository)
// if the user arrived at this dialog from the Protected Branch flow
// we should bypass the "Switch Branch" flow and get out of the user's way
const strategy: UncommittedChangesStrategy = handleProtectedBranchWarning
? {
kind: UncommittedChangesStrategyKind.MoveToNewBranch,
transientStashEntry: null,
}
: askToStash
this.props.dispatcher
.checkoutBranch(this.props.repository, branch)
.checkoutBranch(repository, branch, strategy)
.then(() => timer.done())
}
}
@ -257,10 +275,13 @@ export class BranchesContainer extends React.Component<
}
private onCreateBranchWithName = (name: string) => {
const { repository, handleProtectedBranchWarning } = this.props
this.props.dispatcher.closeFoldout(FoldoutType.Branch)
this.props.dispatcher.showPopup({
type: PopupType.CreateBranch,
repository: this.props.repository,
repository,
handleProtectedBranchWarning,
initialName: name,
})
}

3
app/src/ui/changes/changes-list.tsx
View file

@ -125,6 +125,7 @@ interface IChangesListProps {
readonly dispatcher: Dispatcher
readonly availableWidth: number
readonly isCommitting: boolean
readonly currentBranchProtected: boolean
/**
* Click event handler passed directly to the onRowClick prop of List, see
@ -560,6 +561,7 @@ export class ChangesList extends React.Component<
repository,
dispatcher,
isCommitting,
currentBranchProtected,
} = this.props
if (rebaseConflictState !== null && enablePullWithRebase()) {
@ -615,6 +617,7 @@ export class ChangesList extends React.Component<
)}
singleFileCommit={singleFileCommit}
key={repository.id}
currentBranchProtected={currentBranchProtected}
/>
)
}

21
app/src/ui/changes/commit-message.tsx
View file

@ -23,6 +23,8 @@ import { IAuthor } from '../../models/author'
import { IMenuItem } from '../../lib/menu-item'
import { ICommitContext } from '../../models/commit'
import { startTimer } from '../lib/timing'
import { ProtectedBranchWarning } from './protected-branch-warning'
import { enableBranchProtectionWarningFlow } from '../../lib/feature-flag'
const addAuthorIcon = new OcticonSymbol(
12,
@ -47,6 +49,7 @@ interface ICommitMessageProps {
readonly isCommitting: boolean
readonly placeholder: string
readonly singleFileCommit: boolean
readonly currentBranchProtected: boolean
/**
* Whether or not to show a field for adding co-authors to
@ -439,6 +442,22 @@ export class CommitMessage extends React.Component<
return <div className={className}>{this.renderCoAuthorToggleButton()}</div>
}
private renderProtectedBranchWarning = (branch: string) => {
if (!enableBranchProtectionWarningFlow()) {
return null
}
const { currentBranchProtected, dispatcher } = this.props
if (!currentBranchProtected) {
return null
}
return (
<ProtectedBranchWarning currentBranch={branch} dispatcher={dispatcher} />
)
}
public render() {
const branchName = this.props.branch ? this.props.branch : 'master'
@ -501,6 +520,8 @@ export class CommitMessage extends React.Component<
{this.renderCoAuthorInput()}
{this.renderProtectedBranchWarning(branchName)}
<Button
type="submit"
className="commit-button"

50
app/src/ui/changes/protected-branch-warning.tsx Normal file
View file

@ -0,0 +1,50 @@
import * as React from 'react'
import { Octicon, OcticonSymbol } from '../octicons'
import { LinkButton } from '../lib/link-button'
import { Dispatcher } from '../dispatcher'
import { FoldoutType } from '../../lib/app-state'
interface IProtectedBranchWarningProps {
readonly dispatcher: Dispatcher
readonly currentBranch: string
}
export class ProtectedBranchWarning extends React.Component<
IProtectedBranchWarningProps
> {
private onSwitchBranch = () => {
this.props.dispatcher.showFoldout({
type: FoldoutType.Branch,
handleProtectedBranchWarning: true,
})
}
private ignoreContextMenu = (event: React.MouseEvent<any>) => {
// this prevents the context menu for the root element of CommitMessage from
// firing - it shows 'Add Co-Authors' or 'Remove Co-Authors' based on the
// form state, and for now I'm going to leave that behaviour as-is
// feel free to remove this if that behaviour is revisited
event.preventDefault()
}
public render() {
return (
<div id="protected-branch" onContextMenu={this.ignoreContextMenu}>
<div className="protected-branch-icon-container">
<Octicon
className="protected-branch-icon"
symbol={OcticonSymbol.alert}
/>
</div>
<div className="warning-message">
<strong>{this.props.currentBranch}</strong> is a protected branch.
Want to{' '}
<LinkButton onClick={this.onSwitchBranch}>switch branches</LinkButton>
?
</div>
</div>
)
}
}

2
app/src/ui/changes/sidebar.tsx
View file

@ -354,6 +354,7 @@ export class ChangesSidebar extends React.Component<IChangesSidebarProps, {}> {
coAuthors,
conflictState,
selection,
currentBranchProtected,
} = this.props.changes
// TODO: I think user will expect the avatar to match that which
@ -416,6 +417,7 @@ export class ChangesSidebar extends React.Component<IChangesSidebarProps, {}> {
changesListScrollTop={this.props.changesListScrollTop}
stashEntry={this.props.changes.stashEntry}
isShowingStashEntry={isShowingStashEntry}
currentBranchProtected={currentBranchProtected}
/>
{this.renderUndoCommit(rebaseConflictState)}
</div>

34
app/src/ui/create-branch/create-branch-dialog.tsx
View file

@ -25,6 +25,11 @@ import {
} from '../lib/branch-name-warnings'
import { getStartPoint } from '../../lib/create-branch'
import { startTimer } from '../lib/timing'
import {
UncommittedChangesStrategy,
UncommittedChangesStrategyKind,
askToStash,
} from '../../models/uncommitted-changes-strategy'
interface ICreateBranchProps {
readonly repository: Repository
@ -34,6 +39,9 @@ interface ICreateBranchProps {
readonly defaultBranch: Branch | null
readonly allBranches: ReadonlyArray<Branch>
readonly initialName: string
/** Was this component launched from the "Protected Branch" warning message? */
readonly handleProtectedBranchWarning?: boolean
}
interface ICreateBranchState {
@ -278,26 +286,42 @@ export class CreateBranch extends React.Component<
let startPoint: string | null = null
const {
defaultBranch,
handleProtectedBranchWarning,
repository,
} = this.props
if (this.state.startPoint === StartPoint.DefaultBranch) {
// This really shouldn't happen, we take all kinds of precautions
// to make sure the startPoint state is valid given the current props.
if (!this.props.defaultBranch) {
if (!defaultBranch) {
this.setState({
currentError: new Error('Could not determine the default branch'),
})
return
}
startPoint = this.props.defaultBranch.name
startPoint = defaultBranch.name
}
if (name.length > 0) {
// if the user arrived at this dialog from the Protected Branch flow
// we should bypass the "Switch Branch" flow and get out of the user's way
const strategy: UncommittedChangesStrategy = handleProtectedBranchWarning
? {
kind: UncommittedChangesStrategyKind.MoveToNewBranch,
transientStashEntry: null,
}
: askToStash
this.setState({ isCreatingBranch: true })
const timer = startTimer('create branch', this.props.repository)
const timer = startTimer('create branch', repository)
await this.props.dispatcher.createBranch(
this.props.repository,
repository,
name,
startPoint
startPoint,
strategy
)
timer.done()
}

2
app/src/ui/dispatcher/dispatcher.ts
View file

@ -313,7 +313,7 @@ export class Dispatcher {
return this.appStore._closeCurrentFoldout()
}
/** Close the specified foldout. */
/** Close the specified foldout */
public closeFoldout(foldout: FoldoutType): Promise<void> {
return this.appStore._closeFoldout(foldout)
}

39
app/src/ui/history/compare.tsx
View file

@ -401,35 +401,34 @@ export class CompareSidebar extends React.Component<
event.preventDefault()
return
}
const branch = this.state.focusedBranch
if (this.props.compareState.filterText.length === 0) {
this.handleEscape()
if (branch === null) {
this.viewHistoryForBranch()
} else {
if (this.state.focusedBranch == null) {
this.viewHistoryForBranch()
} else {
const branch = this.state.focusedBranch
this.props.dispatcher.executeCompare(this.props.repository, {
kind: HistoryTabMode.Compare,
comparisonMode: ComparisonMode.Behind,
branch,
})
this.props.dispatcher.executeCompare(this.props.repository, {
kind: HistoryTabMode.Compare,
comparisonMode: ComparisonMode.Behind,
branch,
})
this.props.dispatcher.updateCompareForm(this.props.repository, {
filterText: branch.name,
})
}
this.props.dispatcher.updateCompareForm(this.props.repository, {
filterText: branch.name,
})
}
if (this.textbox) {
this.textbox.blur()
}
if (this.textbox) {
this.textbox.blur()
}
} else if (key === 'Escape') {
this.handleEscape()
} else if (key === 'ArrowDown') {
if (this.branchList !== null) {
this.branchList.selectFirstItem(true)
this.branchList.selectNextItem(true, 'down')
}
} else if (key === 'ArrowUp') {
if (this.branchList !== null) {
this.branchList.selectNextItem(true, 'up')
}
}
}

37
app/src/ui/lib/filter-list.tsx
View file

@ -6,6 +6,7 @@ import {
SelectionSource as ListSelectionSource,
findNextSelectableRow,
ClickSource,
SelectionDirection,
} from '../lib/list'
import { TextBox } from '../lib/text-box'
import { Row } from '../lib/row'
@ -265,19 +266,37 @@ export class FilterList<T extends IFilterListItem> extends React.Component<
)
}
public selectFirstItem(focus: boolean = false) {
public selectNextItem(
focus: boolean = false,
inDirection: SelectionDirection = 'down'
) {
if (this.list === null) {
return
}
let next: number | null = null
const next = findNextSelectableRow(
this.state.rows.length,
{
direction: 'down',
row: -1,
},
this.canSelectRow
)
if (
this.state.selectedRow === -1 ||
this.state.selectedRow === this.state.rows.length
) {
next = findNextSelectableRow(
this.state.rows.length,
{
direction: inDirection,
row: -1,
},
this.canSelectRow
)
} else {
next = findNextSelectableRow(
this.state.rows.length,
{
direction: inDirection,
row: this.state.selectedRow,
},
this.canSelectRow
)
}
if (next !== null) {
this.setState({ selectedRow: next }, () => {

4
app/src/ui/toolbar/branch-dropdown.tsx
View file

@ -42,6 +42,9 @@ interface IBranchDropdownProps {
/** Are we currently loading pull requests? */
readonly isLoadingPullRequests: boolean
/** Was this component launched from the "Protected Branch" warning message? */
readonly handleProtectedBranchWarning?: boolean
}
/**
@ -67,6 +70,7 @@ export class BranchDropdown extends React.Component<IBranchDropdownProps> {
pullRequests={this.props.pullRequests}
currentPullRequest={this.props.currentPullRequest}
isLoadingPullRequests={this.props.isLoadingPullRequests}
handleProtectedBranchWarning={this.props.handleProtectedBranchWarning}
/>
)
}

4
app/src/ui/window/window-controls.tsx
View file

@ -52,9 +52,9 @@ export class WindowControls extends React.Component<{}, IWindowControlState> {
private onWindowStateChanged = (
event: Electron.IpcMessageEvent,
args: any
windowState: WindowState
) => {
this.setState({ windowState: args as WindowState })
this.setState({ windowState })
}
private onMinimize = () => {

1
app/styles/ui/_changes.scss
View file

@ -7,3 +7,4 @@
@import 'changes/no-changes';
@import 'changes/oversized-files-warning';
@import 'changes/blankslate-action';
@import 'changes/protected-branch';

51
app/styles/ui/changes/_protected-branch.scss Normal file
View file

@ -0,0 +1,51 @@
@import '../../mixins';
#protected-branch {
border-top: 1px solid var(--box-border-color);
flex-direction: column;
flex-shrink: 0;
margin-top: auto;
display: flex;
background-color: var(--box-alt-background-color);
padding-top: var(--spacing);
.warning-message {
margin-bottom: 0;
text-align: center;
color: var(--text-secondary-color);
strong {
color: var(--text-color);
word-wrap: break-word;
}
}
.protected-branch-icon-container {
height: 20px;
position: relative;
text-align: center;
.protected-branch-icon {
color: var(--dialog-warning-color);
background: var(--box-alt-background-color);
border-width: 0 var(--spacing-half);
border: solid transparent;
box-sizing: content-box;
position: relative;
z-index: 1;
}
&:after {
display: block;
border-bottom: var(--base-border);
content: '';
position: absolute;
z-index: 0;
display: block;
width: 100%;
top: var(--spacing);
}
}
}

1
app/test/helpers/changes-state-helper.ts
View file

@ -19,6 +19,7 @@ export function createState<K extends keyof IChangesState>(
coAuthors: [],
conflictState: null,
stashEntry: null,
currentBranchProtected: false,
}
return merge(baseChangesState, pick)

124
app/test/helpers/repository-builder-branch-pruner.ts Normal file
View file

@ -0,0 +1,124 @@
import { setupEmptyRepository } from './repositories'
import { makeCommit, switchTo } from './repository-scaffolding'
import { GitProcess } from 'dugite'
import { RepositoriesStore, GitStore } from '../../src/lib/stores'
import { RepositoryStateCache } from '../../src/lib/stores/repository-state-cache'
import { Repository } from '../../src/models/repository'
import { IAPIRepository } from '../../src/lib/api'
import { shell } from './test-app-shell'
export async function createRepository() {
const repo = await setupEmptyRepository()
const firstCommit = {
entries: [
{ path: 'foo', contents: '' },
{ path: 'perlin', contents: 'perlin' },
],
}
await makeCommit(repo, firstCommit)
// creating the new branch before switching so that we have distinct changes
// on both branches and also to ensure a merge commit is needed
await GitProcess.exec(['branch', 'other-branch'], repo.path)
const secondCommit = {
entries: [{ path: 'foo', contents: 'b1' }],
}
await makeCommit(repo, secondCommit)
await switchTo(repo, 'other-branch')
const thirdCommit = {
entries: [{ path: 'bar', contents: 'b2' }],
}
await makeCommit(repo, thirdCommit)
const fourthCommit = {
entries: [{ path: 'baz', contents: 'very much more words' }],
}
await makeCommit(repo, fourthCommit)
await switchTo(repo, 'master')
// ensure the merge operation always creates a merge commit
await GitProcess.exec(['merge', 'other-branch', '--no-ff'], repo.path)
// clear reflog of all entries, so any branches are considered candidates for pruning
await GitProcess.exec(
['reflog', 'expire', '--expire=now', '--expire-unreachable=now', '--all'],
repo.path
)
return repo.path
}
export async function setupRepository(
path: string,
repositoriesStore: RepositoriesStore,
repositoriesStateCache: RepositoryStateCache,
includesGhRepo: boolean,
defaultBranchName: string,
lastPruneDate?: Date
) {
let repository = await repositoriesStore.addRepository(path)
if (includesGhRepo) {
const ghAPIResult: IAPIRepository = {
clone_url: 'string',
ssh_url: 'string',
html_url: 'string',
name: 'string',
owner: {
id: 0,
url: '',
login: '',
avatar_url: '',
type: 'User',
},
private: false,
fork: false,
default_branch: defaultBranchName,
pushed_at: 'string',
parent: null,
}
repository = await repositoriesStore.updateGitHubRepository(
repository,
'',
ghAPIResult
)
}
await primeCaches(repository, repositoriesStateCache)
lastPruneDate &&
repositoriesStore.updateLastPruneDate(repository, lastPruneDate.getTime())
return repository
}
/**
* Setup state correctly without having to expose
* the internals of the GitStore and caches
*/
async function primeCaches(
repository: Repository,
repositoriesStateCache: RepositoryStateCache
) {
const gitStore = new GitStore(repository, shell)
// rather than re-create the branches and stuff as objects, these calls
// will run the underlying Git operations and update the GitStore state
await gitStore.loadRemotes()
await gitStore.loadBranches()
await gitStore.loadStatus()
// once that is done, we can populate the repository state in the same way
// that AppStore does for the sake of this test
repositoriesStateCache.updateBranchesState(repository, () => ({
tip: gitStore.tip,
defaultBranch: gitStore.defaultBranch,
allBranches: gitStore.allBranches,
recentBranches: gitStore.recentBranches,
}))
}

142
app/test/unit/branch-pruner-test.ts
View file

@ -2,14 +2,17 @@ import * as moment from 'moment'
import { BranchPruner } from '../../src/lib/stores/helpers/branch-pruner'
import { Repository } from '../../src/models/repository'
import { GitStoreCache } from '../../src/lib/stores/git-store-cache'
import { RepositoriesStore, GitStore } from '../../src/lib/stores'
import { RepositoriesStore } from '../../src/lib/stores'
import { RepositoryStateCache } from '../../src/lib/stores/repository-state-cache'
import { setupFixtureRepository } from '../helpers/repositories'
import { shell } from '../helpers/test-app-shell'
import { TestRepositoriesDatabase } from '../helpers/databases'
import { GitProcess } from 'dugite'
import { IGitHubUser } from '../../src/lib/databases'
import { IAPIRepository } from '../../src/lib/api'
import {
createRepository as createPrunedRepository,
setupRepository,
} from '../helpers/repository-builder-branch-pruner'
describe('BranchPruner', () => {
const onGitStoreUpdated = () => {}
@ -41,12 +44,16 @@ describe('BranchPruner', () => {
})
it('does nothing on non GitHub repositories', async () => {
const repo = await initializeTestRepo(
const path = await setupFixtureRepository('branch-prune-tests')
const repo = await setupRepository(
path,
repositoriesStore,
repositoriesStateCache,
false,
'master'
)
const branchPruner = new BranchPruner(
repo,
gitStoreCache,
@ -65,7 +72,10 @@ describe('BranchPruner', () => {
it('prunes for GitHub repository', async () => {
const fixedDate = moment()
const lastPruneDate = fixedDate.subtract(1, 'day')
const repo = await initializeTestRepo(
const path = await setupFixtureRepository('branch-prune-tests')
const repo = await setupRepository(
path,
repositoriesStore,
repositoriesStateCache,
true,
@ -83,20 +93,16 @@ describe('BranchPruner', () => {
await branchPruner.start()
const branchesAfterPruning = await getBranchesFromGit(repo)
const expectedBranchesAfterPruning = [
'not-deleted-branch-1',
'deleted-branch-1',
]
for (const branch of expectedBranchesAfterPruning) {
expect(branchesAfterPruning).not.toContain(branch)
}
expect(branchesAfterPruning).not.toContain('deleted-branch-1')
expect(branchesAfterPruning).toContain('not-deleted-branch-1')
})
it('does not prune if the last prune date is less than 24 hours ago', async () => {
const fixedDate = moment()
const lastPruneDate = fixedDate.subtract(4, 'hours')
const repo = await initializeTestRepo(
const path = await setupFixtureRepository('branch-prune-tests')
const repo = await setupRepository(
path,
repositoriesStore,
repositoriesStateCache,
true,
@ -121,7 +127,10 @@ describe('BranchPruner', () => {
it('does not prune if there is no default branch', async () => {
const fixedDate = moment()
const lastPruneDate = fixedDate.subtract(1, 'day')
const repo = await initializeTestRepo(
const path = await setupFixtureRepository('branch-prune-tests')
const repo = await setupRepository(
path,
repositoriesStore,
repositoriesStateCache,
true,
@ -146,7 +155,10 @@ describe('BranchPruner', () => {
it('does not prune reserved branches', async () => {
const fixedDate = moment()
const lastPruneDate = fixedDate.subtract(1, 'day')
const repo = await initializeTestRepo(
const path = await setupFixtureRepository('branch-prune-tests')
const repo = await setupRepository(
path,
repositoriesStore,
repositoriesStateCache,
true,
@ -179,6 +191,36 @@ describe('BranchPruner', () => {
expect(branchesAfterPruning).toContain(branch)
}
})
it('never prunes a branch that lacks an upstream', async () => {
const path = await createPrunedRepository()
const fixedDate = moment()
const lastPruneDate = fixedDate.subtract(1, 'day')
const repo = await setupRepository(
path,
repositoriesStore,
repositoriesStateCache,
true,
'master',
lastPruneDate.toDate()
)
const branchPruner = new BranchPruner(
repo,
gitStoreCache,
repositoriesStore,
repositoriesStateCache,
onPruneCompleted
)
await branchPruner.start()
const branchesAfterPruning = await getBranchesFromGit(repo)
expect(branchesAfterPruning).toContain('master')
expect(branchesAfterPruning).toContain('other-branch')
})
})
async function getBranchesFromGit(repository: Repository) {
@ -188,73 +230,3 @@ async function getBranchesFromGit(repository: Repository) {
.filter(s => s.length > 0)
.map(s => s.substr(2))
}
async function initializeTestRepo(
repositoriesStore: RepositoriesStore,
repositoriesStateCache: RepositoryStateCache,
includesGhRepo: boolean,
defaultBranchName: string,
lastPruneDate?: Date
) {
const path = await setupFixtureRepository('branch-prune-tests')
let repository = await repositoriesStore.addRepository(path)
if (includesGhRepo) {
const ghAPIResult: IAPIRepository = {
clone_url: 'string',
ssh_url: 'string',
html_url: 'string',
name: 'string',
owner: {
id: 0,
url: '',
login: '',
avatar_url: '',
type: 'User',
},
private: false,
fork: false,
default_branch: defaultBranchName,
pushed_at: 'string',
parent: null,
}
repository = await repositoriesStore.updateGitHubRepository(
repository,
'',
ghAPIResult,
[]
)
}
await primeCaches(repository, repositoriesStateCache)
lastPruneDate &&
repositoriesStore.updateLastPruneDate(repository, lastPruneDate.getTime())
return repository
}
/**
* Setup state correctly without having to expose
* the internals of the GitStore and caches
*/
async function primeCaches(
repository: Repository,
repositoriesStateCache: RepositoryStateCache
) {
const gitStore = new GitStore(repository, shell)
// rather than re-create the branches and stuff as objects, these calls
// will run the underlying Git operations and update the GitStore state
await gitStore.loadRemotes()
await gitStore.loadBranches()
await gitStore.loadStatus()
// once that is done, we can populate the repository state in the same way
// that AppStore does for the sake of this test
repositoriesStateCache.updateBranchesState(repository, () => ({
tip: gitStore.tip,
defaultBranch: gitStore.defaultBranch,
allBranches: gitStore.allBranches,
recentBranches: gitStore.recentBranches,
}))
}

9
app/test/unit/repositories-store-test.ts
View file

@ -60,8 +60,7 @@ describe('RepositoriesStore', () => {
await repositoriesStore!.updateGitHubRepository(
addedRepo,
'https://api.github.com',
gitHubRepo,
[]
gitHubRepo
)
const repositories = await repositoriesStore!.getAll()
@ -80,8 +79,7 @@ describe('RepositoriesStore', () => {
const updatedFirstRepo = await repositoriesStore!.updateGitHubRepository(
firstRepo,
'https://api.github.com',
gitHubRepo,
[]
gitHubRepo
)
const secondRepo = await repositoriesStore!.addRepository(
@ -90,8 +88,7 @@ describe('RepositoriesStore', () => {
const updatedSecondRepo = await repositoriesStore!.updateGitHubRepository(
secondRepo,
'https://api.github.com',
gitHubRepo,
[]
gitHubRepo
)
expect(updatedFirstRepo.gitHubRepository!.dbID).toBe(

12
changelog.json
View file

@ -1,5 +1,17 @@
{
"releases": {
"2.1.0-beta1": [
"[New] Commit form will warn user when working on a protected branch - #7826",
"[Added] Keyboard shortcut for \"Discard All Changes\" menu item - #7588. Thanks @say25!",
"[Fixed] New repository does not write description into README - #7571. Thanks @noelledusahel!",
"[Fixed] Disable \"Discard\" and \"Restore\" buttons while restoring stash - #7289",
"[Fixed] \"Unable to restore\" warning message appears when restoring stash - #7652",
"[Fixed] Unresponsive app on macOS if user switches away from file dialog - #7636",
"[Fixed] Launching app on Windows after being maximized does not restore correct window state - #7750",
"[Improved] Update mentions of \"Enterprise\" to \"Enterprise Server\" in app - #7728 #7845 #7835. Thanks @nathos!",
"[Improved] Update license and .gitignore templates for initializing a new repository - #7548 #7661. Thanks @VADS!",
"[Improved] \"Authentication failed\" dialog provides more help to diagnose issue - #7622"
],
"2.0.4-test2": ["Upgrading infrastructure to Node 10"],
"2.0.4": [
"[Fixed] Refresh for Enterprise repositories did not handle API error querying branches - #7713",