diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000000..ca79ca5b4d --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,6 @@ +version: 2 +updates: + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 40e7a8537d..b4e11559d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,11 +28,11 @@ jobs: # Needed for macOS arm64 until hosted macos-11.0 runners become available SDKROOT: /Library/Developer/CommandLineTools/SDKs/MacOSX11.1.sdk steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: submodules: recursive - name: Use Node.js ${{ matrix.node }} - uses: actions/setup-node@v1 + uses: actions/setup-node@v3 with: node-version: ${{ matrix.node }} @@ -40,7 +40,7 @@ jobs: id: yarn-cache-dir-path run: echo "::set-output name=dir::$(yarn cache dir)" - - uses: actions/cache@v2 + - uses: actions/cache@v3 id: yarn-cache # use this to check for `cache-hit` (`steps.yarn-cache.outputs.cache-hit != 'true'`) with: path: ${{ steps.yarn-cache-dir-path.outputs.dir }} diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 4d8d7d8f23..0118ec3438 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -18,11 +18,11 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v2 + uses: actions/checkout@v3 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@v1 + uses: github/codeql-action/init@v2 with: config-file: ./.github/codeql/codeql-config.yml @@ -32,7 +32,7 @@ jobs: # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). # If this step fails, then you should remove it and run the build manually (see below). - name: Autobuild - uses: github/codeql-action/autobuild@v1 + uses: github/codeql-action/autobuild@v2 # ℹ️ Command-line programs to run using the OS shell. # 📚 https://git.io/JvXDl @@ -46,4 +46,4 @@ jobs: # make release - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 + uses: github/codeql-action/analyze@v2 diff --git a/.github/workflows/release-pr.yml b/.github/workflows/release-pr.yml index 3388c687dd..0619858d7b 100644 --- a/.github/workflows/release-pr.yml +++ b/.github/workflows/release-pr.yml @@ -9,7 +9,7 @@ jobs: permissions: pull-requests: write steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 if: | startsWith(github.ref, 'refs/heads/releases/') && !contains(github.ref, 'test') diff --git a/README.md b/README.md index a5038a7fdb..d8909ee13e 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # [GitHub Desktop](https://desktop.github.com) [GitHub Desktop](https://desktop.github.com/) is an open source [Electron](https://www.electronjs.org/)-based -GitHub app. It is written in [TypeScript](http://www.typescriptlang.org) and +GitHub app. It is written in [TypeScript](https://www.typescriptlang.org) and uses [React](https://reactjs.org/). ![GitHub Desktop screenshot - Windows](https://cloud.githubusercontent.com/assets/359239/26094502/a1f56d02-3a5d-11e7-8799-23c7ba5e5106.png) diff --git a/app/package.json b/app/package.json index f3f7d1f37e..5de3e13118 100644 --- a/app/package.json +++ b/app/package.json @@ -26,7 +26,7 @@ "codemirror-mode-elixir": "^1.1.2", "compare-versions": "^3.6.0", "deep-equal": "^1.0.1", - "desktop-notifications": "^0.2.2", + "desktop-notifications": "^0.2.4", "desktop-trampoline": "desktop/desktop-trampoline#v0.9.8", "dexie": "^3.2.2", "dompurify": "^2.3.3", diff --git a/app/src/lib/app-state.ts b/app/src/lib/app-state.ts index d02694c324..92e068ce9b 100644 --- a/app/src/lib/app-state.ts +++ b/app/src/lib/app-state.ts @@ -553,6 +553,25 @@ export interface ICommitSelection { /** The commits currently selected in the app */ readonly shas: ReadonlyArray + /** + * When multiple commits are selected, the diff is created using the rev range + * of firstSha^..lastSha in the selected shas. Thus comparing the trees of the + * the lastSha and the first parent of the first sha. However, our history + * list shows commits in chronological order. Thus, when a branch is merged, + * the commits from that branch are injected in their chronological order into + * the history list. Therefore, given a branch history of A, B, C, D, + * MergeCommit where B and C are from the merged branch, diffing on the + * selection of A through D would not have the changes from B an C. + * + * This is a list of the shas that are reachable by following the parent links + * (aka the graph) from the lastSha to the firstSha^ in the selection. + * + * Other notes: Given a selection A through D, executing `git diff A..D` would + * give us the changes since A but not including A; since the user will have + * selected A, we do `git diff A^..D` so that we include the changes of A. + * */ + readonly shasInDiff: ReadonlyArray + /** * Whether the a selection of commits are group of adjacent to each other. * Example: Given these are indexes of sha's in history, 3, 4, 5, 6 is contiguous as @@ -717,6 +736,9 @@ export interface ICompareState { /** The SHAs of commits to render in the compare list */ readonly commitSHAs: ReadonlyArray + /** The SHAs of commits to highlight in the compare list */ + readonly shasToHighlight: ReadonlyArray + /** * A list of branches (remote and local) except the current branch, and * Desktop fork remote branches (see `Branch.isDesktopForkRemoteBranch`) diff --git a/app/src/lib/branch.ts b/app/src/lib/branch.ts index 93ee13b937..052895dd8f 100644 --- a/app/src/lib/branch.ts +++ b/app/src/lib/branch.ts @@ -3,7 +3,10 @@ import { UpstreamRemoteName } from './stores' import { RepositoryWithGitHubRepository, getNonForkGitHubRepository, + isRepositoryWithGitHubRepository, + Repository, } from '../models/repository' +import { IBranchesState } from './app-state' /** * Finds the default branch of the upstream repository of the passed repository. @@ -41,3 +44,23 @@ export function findDefaultUpstreamBranch( return foundBranch !== undefined ? foundBranch : null } + +/** + * + * @param repository The repository to use. + * @param branchesState The branches state of the repository. + * @returns The default branch of the user's contribution target, or null if it's not known. + * + * This method will return the fork's upstream default branch, if the user + * is contributing to the parent repository. + * + * Otherwise, this method will return the default branch of the passed in repository. + */ +export function findContributionTargetDefaultBranch( + repository: Repository, + { allBranches, defaultBranch }: IBranchesState +): Branch | null { + return isRepositoryWithGitHubRepository(repository) + ? findDefaultUpstreamBranch(repository, allBranches) ?? defaultBranch + : defaultBranch +} diff --git a/app/src/lib/menu-update.ts b/app/src/lib/menu-update.ts index 0c293cefcd..1508d43c07 100644 --- a/app/src/lib/menu-update.ts +++ b/app/src/lib/menu-update.ts @@ -112,7 +112,7 @@ const allMenuIds: ReadonlyArray = [ 'discard-all-changes', 'stash-all-changes', 'preferences', - 'update-branch', + 'update-branch-with-contribution-target-branch', 'compare-to-branch', 'merge-branch', 'rebase-branch', @@ -261,7 +261,7 @@ function getRepositoryMenuBuilder(state: IAppState): MenuStateBuilder { onNonDefaultBranch && !branchIsUnborn && !onDetachedHead ) menuStateBuilder.setEnabled( - 'update-branch', + 'update-branch-with-contribution-target-branch', onNonDefaultBranch && hasDefaultBranch && !onDetachedHead ) menuStateBuilder.setEnabled('merge-branch', onBranch) @@ -343,7 +343,7 @@ function getRepositoryMenuBuilder(state: IAppState): MenuStateBuilder { menuStateBuilder.disable('delete-branch') menuStateBuilder.disable('discard-all-changes') menuStateBuilder.disable('stash-all-changes') - menuStateBuilder.disable('update-branch') + menuStateBuilder.disable('update-branch-with-contribution-target-branch') menuStateBuilder.disable('merge-branch') if (enableSquashMerging()) { menuStateBuilder.disable('squash-and-merge-branch') diff --git a/app/src/lib/shells/darwin.ts b/app/src/lib/shells/darwin.ts index f9f482d3db..4140cd45d9 100644 --- a/app/src/lib/shells/darwin.ts +++ b/app/src/lib/shells/darwin.ts @@ -12,6 +12,7 @@ export enum Shell { Kitty = 'Kitty', Alacritty = 'Alacritty', WezTerm = 'WezTerm', + Warp = 'Warp', } export const Default = Shell.Terminal @@ -36,6 +37,8 @@ function getBundleID(shell: Shell): string { return 'io.alacritty' case Shell.WezTerm: return 'com.github.wez.wezterm' + case Shell.Warp: + return 'dev.warp.Warp-Stable' default: return assertNever(shell, `Unknown shell: ${shell}`) } @@ -62,6 +65,7 @@ export async function getAvailableShells(): Promise< kittyPath, alacrittyPath, wezTermPath, + warpPath, ] = await Promise.all([ getShellPath(Shell.Terminal), getShellPath(Shell.Hyper), @@ -70,6 +74,7 @@ export async function getAvailableShells(): Promise< getShellPath(Shell.Kitty), getShellPath(Shell.Alacritty), getShellPath(Shell.WezTerm), + getShellPath(Shell.Warp), ]) const shells: Array> = [] @@ -104,6 +109,11 @@ export async function getAvailableShells(): Promise< shells.push({ shell: Shell.WezTerm, path: wezTermExecutable }) } + if (warpPath) { + const warpExecutable = `${warpPath}/Contents/MacOS/stable` + shells.push({ shell: Shell.Warp, path: warpExecutable }) + } + return shells } diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 3bc0ff1659..df80b9027e 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -300,6 +300,7 @@ import { import * as ipcRenderer from '../ipc-renderer' import { pathExists } from '../../ui/lib/path-exists' import { offsetFromNow } from '../offset-from' +import { findContributionTargetDefaultBranch } from '../branch' import { ValidNotificationPullRequestReview } from '../valid-notification-pull-request-review' import { determineMergeability } from '../git/merge-tree' @@ -1109,12 +1110,13 @@ export class AppStore extends TypedBaseStore { } /** This shouldn't be called directly. See `Dispatcher`. */ - public async _changeCommitSelection( + public _changeCommitSelection( repository: Repository, shas: ReadonlyArray, isContiguous: boolean - ): Promise { - const { commitSelection } = this.repositoryStateCache.get(repository) + ): void { + const { commitSelection, commitLookup } = + this.repositoryStateCache.get(repository) if ( commitSelection.shas.length === shas.length && @@ -1123,8 +1125,11 @@ export class AppStore extends TypedBaseStore { return } + const shasInDiff = this.getShasInDiff(shas, isContiguous, commitLookup) + this.repositoryStateCache.updateCommitSelection(repository, () => ({ shas, + shasInDiff, isContiguous, file: null, changesetData: { files: [], linesAdded: 0, linesDeleted: 0 }, @@ -1134,6 +1139,65 @@ export class AppStore extends TypedBaseStore { this.emitUpdate() } + /** This shouldn't be called directly. See `Dispatcher`. */ + public async _updateShasToHighlight( + repository: Repository, + shasToHighlight: ReadonlyArray + ) { + this.repositoryStateCache.updateCompareState(repository, () => ({ + shasToHighlight, + })) + this.emitUpdate() + } + + /** + * When multiple commits are selected, the diff is created using the rev range + * of firstSha^..lastSha in the selected shas. Thus comparing the trees of the + * the lastSha and the first parent of the first sha. However, our history + * list shows commits in chronological order. Thus, when a branch is merged, + * the commits from that branch are injected in their chronological order into + * the history list. Therefore, given a branch history of A, B, C, D, + * MergeCommit where B and C are from the merged branch, diffing on the + * selection of A through D would not have the changes from B an C. + * + * This method traverses the ancestral path from the last commit in the + * selection back to the first commit via checking the parents. The + * commits on this path are the commits whose changes will be seen in the + * diff. This is equivalent to doing `git rev-list firstSha^..lastSha`. + */ + private getShasInDiff( + selectedShas: ReadonlyArray, + isContiguous: boolean, + commitLookup: Map + ) { + const shasInDiff = new Array() + + if (selectedShas.length <= 1 || !isContiguous) { + return selectedShas + } + + const shasToTraverse = [selectedShas.at(-1)] + do { + const currentSha = shasToTraverse.pop() + if (currentSha === undefined) { + continue + } + + shasInDiff.push(currentSha) + + // shas are selection of history -> should be in lookup -> `|| []` is for typing sake + const parentSHAs = commitLookup.get(currentSha)?.parentSHAs || [] + + const parentsInSelection = parentSHAs.filter(parentSha => + selectedShas.includes(parentSha) + ) + + shasToTraverse.push(...parentsInSelection) + } while (shasToTraverse.length > 0) + + return shasInDiff + } + private updateOrSelectFirstCommit( repository: Repository, commitSHAs: ReadonlyArray @@ -2036,6 +2100,7 @@ export class AppStore extends TypedBaseStore { private updateMenuItemLabels(state: IRepositoryState | null) { const { selectedShell, + selectedRepository, selectedExternalEditor, askForConfirmationOnRepositoryRemoval, askForConfirmationOnForcePush, @@ -2054,12 +2119,14 @@ export class AppStore extends TypedBaseStore { } const { changesState, branchesState, aheadBehind } = state - const { defaultBranch, currentPullRequest } = branchesState + const { currentPullRequest } = branchesState - const defaultBranchName = - defaultBranch === null || defaultBranch.upstreamWithoutRemote === null - ? undefined - : defaultBranch.upstreamWithoutRemote + let contributionTargetDefaultBranch: string | undefined + if (selectedRepository instanceof Repository) { + contributionTargetDefaultBranch = + findContributionTargetDefaultBranch(selectedRepository, branchesState) + ?.name ?? undefined + } const isForcePushForCurrentRepository = isCurrentBranchForcePush( branchesState, @@ -2074,7 +2141,7 @@ export class AppStore extends TypedBaseStore { updatePreferredAppMenuItemLabels({ ...labels, - defaultBranchName, + contributionTargetDefaultBranch, isForcePushForCurrentRepository, isStashedChangesVisible, hasCurrentPullRequest: currentPullRequest !== null, diff --git a/app/src/lib/stores/repository-state-cache.ts b/app/src/lib/stores/repository-state-cache.ts index d7e2586657..fcba3c82c3 100644 --- a/app/src/lib/stores/repository-state-cache.ts +++ b/app/src/lib/stores/repository-state-cache.ts @@ -178,6 +178,7 @@ function getInitialRepositoryState(): IRepositoryState { return { commitSelection: { shas: [], + shasInDiff: [], isContiguous: true, file: null, changesetData: { files: [], linesAdded: 0, linesDeleted: 0 }, @@ -219,6 +220,7 @@ function getInitialRepositoryState(): IRepositoryState { showBranchList: false, filterText: '', commitSHAs: [], + shasToHighlight: [], branches: new Array(), recentBranches: new Array(), defaultBranch: null, diff --git a/app/src/main-process/app-window.ts b/app/src/main-process/app-window.ts index 54fff826f0..073509acac 100644 --- a/app/src/main-process/app-window.ts +++ b/app/src/main-process/app-window.ts @@ -25,6 +25,7 @@ import { installNotificationCallback, terminateDesktopNotifications, } from './notifications' +import { addTrustedIPCSender } from './trusted-ipc-sender' export class AppWindow { private window: Electron.BrowserWindow @@ -77,6 +78,7 @@ export class AppWindow { } this.window = new BrowserWindow(windowOptions) + addTrustedIPCSender(this.window.webContents) installNotificationCallback(this.window) diff --git a/app/src/main-process/crash-window.ts b/app/src/main-process/crash-window.ts index 3a934d0589..d91fee11ef 100644 --- a/app/src/main-process/crash-window.ts +++ b/app/src/main-process/crash-window.ts @@ -4,6 +4,7 @@ import { ICrashDetails, ErrorType } from '../crash/shared' import { registerWindowStateChangedEvents } from '../lib/window-state' import * as ipcMain from './ipc-main' import * as ipcWebContents from './ipc-webcontents' +import { addTrustedIPCSender } from './trusted-ipc-sender' const minWidth = 600 const minHeight = 500 @@ -51,6 +52,7 @@ export class CrashWindow { } this.window = new BrowserWindow(windowOptions) + addTrustedIPCSender(this.window.webContents) this.error = error this.errorType = errorType diff --git a/app/src/main-process/ipc-main.ts b/app/src/main-process/ipc-main.ts index 1260ccfa25..a68f4c99f5 100644 --- a/app/src/main-process/ipc-main.ts +++ b/app/src/main-process/ipc-main.ts @@ -2,6 +2,17 @@ import { RequestChannels, RequestResponseChannels } from '../lib/ipc-shared' // eslint-disable-next-line no-restricted-imports import { ipcMain } from 'electron' import { IpcMainEvent, IpcMainInvokeEvent } from 'electron/main' +import { isTrustedIPCSender } from './trusted-ipc-sender' + +type RequestChannelListener = ( + event: IpcMainEvent, + ...args: Parameters +) => void + +type RequestResponseChannelListener = ( + event: IpcMainInvokeEvent, + ...args: Parameters +) => ReturnType /** * Subscribes to the specified IPC channel and provides strong typing of @@ -10,12 +21,9 @@ import { IpcMainEvent, IpcMainInvokeEvent } from 'electron/main' */ export function on( channel: T, - listener: ( - event: IpcMainEvent, - ...args: Parameters - ) => void + listener: RequestChannelListener ) { - ipcMain.on(channel, (event, ...args) => listener(event, ...(args as any))) + ipcMain.on(channel, safeListener(listener)) } /** @@ -25,12 +33,9 @@ export function on( */ export function once( channel: T, - listener: ( - event: IpcMainEvent, - ...args: Parameters - ) => void + listener: RequestChannelListener ) { - ipcMain.once(channel, (event, ...args) => listener(event, ...(args as any))) + ipcMain.once(channel, safeListener(listener)) } /** @@ -40,10 +45,22 @@ export function once( */ export function handle( channel: T, - listener: ( - event: IpcMainInvokeEvent, - ...args: Parameters - ) => ReturnType + listener: RequestResponseChannelListener ) { - ipcMain.handle(channel, (event, ...args) => listener(event, ...(args as any))) + ipcMain.handle(channel, safeListener(listener)) +} + +function safeListener( + listener: (event: E, ...a: any) => R +) { + return (event: E, ...args: any) => { + if (!isTrustedIPCSender(event.sender)) { + log.error( + `IPC message received from invalid sender: ${event.senderFrame.url}` + ) + return + } + + return listener(event, ...args) + } } diff --git a/app/src/main-process/menu/build-default-menu.ts b/app/src/main-process/menu/build-default-menu.ts index a79f1dba50..456bbc0649 100644 --- a/app/src/main-process/menu/build-default-menu.ts +++ b/app/src/main-process/menu/build-default-menu.ts @@ -38,12 +38,15 @@ export function buildDefaultMenu({ askForConfirmationOnForcePush, askForConfirmationOnRepositoryRemoval, hasCurrentPullRequest = false, - defaultBranchName = defaultBranchNameValue, + contributionTargetDefaultBranch = defaultBranchNameValue, isForcePushForCurrentRepository = false, isStashedChangesVisible = false, askForConfirmationWhenStashingAllChanges = true, }: MenuLabelsEvent): Electron.Menu { - defaultBranchName = truncateWithEllipsis(defaultBranchName, 25) + contributionTargetDefaultBranch = truncateWithEllipsis( + contributionTargetDefaultBranch, + 25 + ) const removeRepoLabel = askForConfirmationOnRepositoryRemoval ? confirmRepositoryRemovalLabel @@ -376,11 +379,11 @@ export function buildDefaultMenu({ separator, { label: __DARWIN__ - ? `Update from ${defaultBranchName}` - : `&Update from ${defaultBranchName}`, - id: 'update-branch', + ? `Update from ${contributionTargetDefaultBranch}` + : `&Update from ${contributionTargetDefaultBranch}`, + id: 'update-branch-with-contribution-target-branch', accelerator: 'CmdOrCtrl+Shift+U', - click: emit('update-branch'), + click: emit('update-branch-with-contribution-target-branch'), }, { label: __DARWIN__ ? 'Compare to Branch' : '&Compare to branch', diff --git a/app/src/main-process/menu/menu-event.ts b/app/src/main-process/menu/menu-event.ts index 561b7fb438..651cf994f6 100644 --- a/app/src/main-process/menu/menu-event.ts +++ b/app/src/main-process/menu/menu-event.ts @@ -16,7 +16,7 @@ export type MenuEvent = | 'show-preferences' | 'choose-repository' | 'open-working-directory' - | 'update-branch' + | 'update-branch-with-contribution-target-branch' | 'compare-to-branch' | 'merge-branch' | 'squash-and-merge-branch' diff --git a/app/src/main-process/notifications.ts b/app/src/main-process/notifications.ts index 98717e04d0..0ae209ca46 100644 --- a/app/src/main-process/notifications.ts +++ b/app/src/main-process/notifications.ts @@ -11,6 +11,11 @@ import * as ipcWebContents from './ipc-webcontents' let windowsToastActivatorClsid: string | undefined = undefined export function initializeDesktopNotifications() { + if (__LINUX__) { + // notifications not currently supported + return + } + if (__DARWIN__) { initializeNotifications({}) return @@ -24,7 +29,7 @@ export function initializeDesktopNotifications() { if (windowsToastActivatorClsid === undefined) { log.error( - 'Toast activator CLSID not found in any of the shortucts. Falling back to known CLSIDs.' + 'Toast activator CLSID not found in any of the shortcuts. Falling back to known CLSIDs.' ) // This is generated by Squirrel.Windows here: diff --git a/app/src/main-process/show-uncaught-exception.ts b/app/src/main-process/show-uncaught-exception.ts index e5b5c3151c..5f04ee66ff 100644 --- a/app/src/main-process/show-uncaught-exception.ts +++ b/app/src/main-process/show-uncaught-exception.ts @@ -17,16 +17,13 @@ export function showUncaughtException(isLaunchError: boolean, error: Error) { setCrashMenu() - const crashWindow = new CrashWindow( - isLaunchError ? 'launch' : 'generic', - error - ) + const window = new CrashWindow(isLaunchError ? 'launch' : 'generic', error) - crashWindow.onDidLoad(() => { - crashWindow.show() + window.onDidLoad(() => { + window.show() }) - crashWindow.onFailedToLoad(async () => { + window.onFailedToLoad(async () => { await dialog.showMessageBox({ type: 'error', title: __DARWIN__ ? `Unrecoverable Error` : 'Unrecoverable error', @@ -44,12 +41,12 @@ export function showUncaughtException(isLaunchError: boolean, error: Error) { app.quit() }) - crashWindow.onClose(() => { + window.onClose(() => { if (!__DEV__) { app.relaunch() } app.quit() }) - crashWindow.load() + window.load() } diff --git a/app/src/main-process/trusted-ipc-sender.ts b/app/src/main-process/trusted-ipc-sender.ts new file mode 100644 index 0000000000..ae36a90e78 --- /dev/null +++ b/app/src/main-process/trusted-ipc-sender.ts @@ -0,0 +1,16 @@ +import { WebContents } from 'electron' + +// WebContents id of trusted senders of IPC messages. This is used to verify +// that only IPC messages sent from trusted senders are handled, as recommended +// by the Electron security documentation: +// https://github.com/electron/electron/blob/main/docs/tutorial/security.md#17-validate-the-sender-of-all-ipc-messages +const trustedSenders = new Set() + +/** Adds a WebContents instance to the set of trusted IPC senders. */ +export const addTrustedIPCSender = (wc: WebContents) => { + trustedSenders.add(wc.id) + wc.on('destroyed', () => trustedSenders.delete(wc.id)) +} + +/** Returns true if the given WebContents is a trusted sender of IPC messages. */ +export const isTrustedIPCSender = (wc: WebContents) => trustedSenders.has(wc.id) diff --git a/app/src/models/menu-ids.ts b/app/src/models/menu-ids.ts index 63b035e6e4..96157b9788 100644 --- a/app/src/models/menu-ids.ts +++ b/app/src/models/menu-ids.ts @@ -5,7 +5,7 @@ export type MenuIDs = | 'discard-all-changes' | 'stash-all-changes' | 'preferences' - | 'update-branch' + | 'update-branch-with-contribution-target-branch' | 'merge-branch' | 'squash-and-merge-branch' | 'rebase-branch' diff --git a/app/src/models/menu-labels.ts b/app/src/models/menu-labels.ts index 974297d61d..1093e0c474 100644 --- a/app/src/models/menu-labels.ts +++ b/app/src/models/menu-labels.ts @@ -28,11 +28,16 @@ export type MenuLabelsEvent = { readonly askForConfirmationOnRepositoryRemoval: boolean /** - * Specify the default branch associated with the current repository. + * Specify the default branch of the user's contribution target. + * + * This value should be the fork's upstream default branch, if the user + * is contributing to the parent repository. + * + * Otherwise, this value should be the default branch of the repository. * * Omit this value to indicate that the default branch is unknown. */ - readonly defaultBranchName?: string + readonly contributionTargetDefaultBranch?: string /** * Is the current branch in a state where it can be force pushed to the remote? diff --git a/app/src/models/popup.ts b/app/src/models/popup.ts index fdf1058b3a..ed3e613e3a 100644 --- a/app/src/models/popup.ts +++ b/app/src/models/popup.ts @@ -21,6 +21,7 @@ import { IAuthor } from './author' import { IRefCheck } from '../lib/ci-checks/ci-checks' import { GitHubRepository } from './github-repository' import { ValidNotificationPullRequestReview } from '../lib/valid-notification-pull-request-review' +import { UnreachableCommitsTab } from '../ui/history/unreachable-commits-dialog' export enum PopupType { RenameBranch = 1, @@ -84,6 +85,7 @@ export enum PopupType { WarnForcePush, DiscardChangesRetry, PullRequestReview, + UnreachableCommits, } export type Popup = @@ -353,3 +355,7 @@ export type Popup = shouldCheckoutBranch: boolean shouldChangeRepository: boolean } + | { + type: PopupType.UnreachableCommits + selectedTab: UnreachableCommitsTab + } diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index 6140020c25..f253d33a04 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -104,7 +104,10 @@ import { TutorialStep, isValidTutorialStep } from '../models/tutorial-step' import { WorkflowPushRejectedDialog } from './workflow-push-rejected/workflow-push-rejected' import { SAMLReauthRequiredDialog } from './saml-reauth-required/saml-reauth-required' import { CreateForkDialog } from './forks/create-fork-dialog' -import { findDefaultUpstreamBranch } from '../lib/branch' +import { + findContributionTargetDefaultBranch, + findDefaultUpstreamBranch, +} from '../lib/branch' import { GitHubRepository, hasWritePermission, @@ -147,6 +150,7 @@ import { PullRequestChecksFailed } from './notifications/pull-request-checks-fai import { CICheckRunRerunDialog } from './check-runs/ci-check-run-rerun-dialog' import { WarnForcePushDialog } from './multi-commit-operation/dialog/warn-force-push-dialog' import { clamp } from '../lib/clamp' +import { generateRepositoryListContextMenu } from './repositories-list/repository-list-item-context-menu' import * as ipcRenderer from '../lib/ipc-renderer' import { showNotification } from '../lib/notifications/show-notification' import { DiscardChangesRetryDialog } from './discard-changes/discard-changes-retry-dialog' @@ -155,6 +159,8 @@ import { PullRequestReview } from './notifications/pull-request-review' import { getPullRequestCommitRef } from '../models/pull-request' import { getRepositoryType } from '../lib/git' import { SSHUserPassword } from './ssh/ssh-user-password' +import { showContextualMenu } from '../lib/menu-item' +import { UnreachableCommitsDialog } from './history/unreachable-commits-dialog' const MinuteInMilliseconds = 1000 * 60 const HourInMilliseconds = MinuteInMilliseconds * 60 @@ -382,9 +388,9 @@ export class App extends React.Component { return this.props.dispatcher.showPopup({ type: PopupType.Preferences }) case 'open-working-directory': return this.openCurrentRepositoryWorkingDirectory() - case 'update-branch': + case 'update-branch-with-contribution-target-branch': this.props.dispatcher.recordMenuInitiatedUpdate() - return this.updateBranch() + return this.updateBranchWithContributionTargetBranch() case 'compare-to-branch': return this.showHistory(true) case 'merge-branch': @@ -635,7 +641,7 @@ export class App extends React.Component { return enterpriseAccount || null } - private updateBranch() { + private updateBranchWithContributionTargetBranch() { const { selectedState } = this.state if ( selectedState == null || @@ -645,19 +651,27 @@ export class App extends React.Component { } const { state, repository } = selectedState - const defaultBranch = state.branchesState.defaultBranch - if (!defaultBranch) { + + const contributionTargetDefaultBranch = findContributionTargetDefaultBranch( + repository, + state.branchesState + ) + if (!contributionTargetDefaultBranch) { return } this.props.dispatcher.initializeMergeOperation( repository, false, - defaultBranch + contributionTargetDefaultBranch ) const { mergeStatus } = state.compareState - this.props.dispatcher.mergeBranch(repository, defaultBranch, mergeStatus) + this.props.dispatcher.mergeBranch( + repository, + contributionTargetDefaultBranch, + mergeStatus + ) } private mergeBranch(isSquash: boolean = false) { @@ -2201,6 +2215,31 @@ export class App extends React.Component { /> ) } + case PopupType.UnreachableCommits: { + const { selectedState, emoji } = this.state + if ( + selectedState == null || + selectedState.type !== SelectionType.Repository + ) { + return null + } + + const { + commitLookup, + commitSelection: { shas, shasInDiff }, + } = selectedState.state + + return ( + + ) + } default: return assertNever(popup, `Unknown popup type: ${popup}`) } @@ -2555,6 +2594,7 @@ export class App extends React.Component { description={__DARWIN__ ? 'Current Repository' : 'Current repository'} tooltip={tooltip} foldoutStyle={foldoutStyle} + onContextMenu={this.onRepositoryToolbarButtonContextMenu} onDropdownStateChanged={this.onRepositoryDropdownStateChanged} dropdownContentRenderer={this.renderRepositoryList} dropdownState={currentState} @@ -2562,6 +2602,43 @@ export class App extends React.Component { ) } + private onRepositoryToolbarButtonContextMenu = () => { + const repository = this.state.selectedState?.repository + if (repository === undefined) { + return + } + + const externalEditorLabel = this.state.selectedExternalEditor ?? undefined + + const onChangeRepositoryAlias = (repository: Repository) => { + this.props.dispatcher.showPopup({ + type: PopupType.ChangeRepositoryAlias, + repository, + }) + } + + const onRemoveRepositoryAlias = (repository: Repository) => { + this.props.dispatcher.changeRepositoryAlias(repository, null) + } + + const items = generateRepositoryListContextMenu({ + onRemoveRepository: this.removeRepository, + onShowRepository: this.showRepository, + onOpenInShell: this.openInShell, + onOpenInExternalEditor: this.openInExternalEditor, + askForConfirmationOnRemoveRepository: + this.state.askForConfirmationOnRepositoryRemoval, + externalEditorLabel: externalEditorLabel, + onChangeRepositoryAlias: onChangeRepositoryAlias, + onRemoveRepositoryAlias: onRemoveRepositoryAlias, + onViewOnGitHub: this.viewOnGitHub, + repository: repository, + shellLabel: this.state.selectedShell, + }) + + showContextualMenu(items) + } + private renderPushPullToolbarButton() { const selection = this.state.selectedState if (!selection || selection.type !== SelectionType.Repository) { diff --git a/app/src/ui/branches/branch-list-item-context-menu.tsx b/app/src/ui/branches/branch-list-item-context-menu.tsx new file mode 100644 index 0000000000..88334e3044 --- /dev/null +++ b/app/src/ui/branches/branch-list-item-context-menu.tsx @@ -0,0 +1,40 @@ +import { IMenuItem } from '../../lib/menu-item' +import { clipboard } from 'electron' + +interface IBranchContextMenuConfig { + name: string + isLocal: boolean + onRenameBranch?: (branchName: string) => void + onDeleteBranch?: (branchName: string) => void +} + +export function generateBranchContextMenuItems( + config: IBranchContextMenuConfig +): IMenuItem[] { + const { name, isLocal, onRenameBranch, onDeleteBranch } = config + const items = new Array() + + if (onRenameBranch !== undefined) { + items.push({ + label: 'Rename…', + action: () => onRenameBranch(name), + enabled: isLocal, + }) + } + + items.push({ + label: __DARWIN__ ? 'Copy Branch Name' : 'Copy branch name', + action: () => clipboard.writeText(name), + }) + + items.push({ type: 'separator' }) + + if (onDeleteBranch !== undefined) { + items.push({ + label: 'Delete…', + action: () => onDeleteBranch(name), + }) + } + + return items +} diff --git a/app/src/ui/branches/branch-list-item.tsx b/app/src/ui/branches/branch-list-item.tsx index 54cc1c3af0..27dc741f10 100644 --- a/app/src/ui/branches/branch-list-item.tsx +++ b/app/src/ui/branches/branch-list-item.tsx @@ -1,4 +1,3 @@ -import { clipboard } from 'electron' import * as React from 'react' import { IMatches } from '../../lib/fuzzy-find' @@ -7,12 +6,12 @@ import { Octicon } from '../octicons' import * as OcticonSymbol from '../octicons/octicons.generated' import { HighlightText } from '../lib/highlight-text' import { showContextualMenu } from '../../lib/menu-item' -import { IMenuItem } from '../../lib/menu-item' import { dragAndDropManager } from '../../lib/drag-and-drop-manager' import { DragType, DropTargetType } from '../../models/drag-drop' import { TooltippedContent } from '../lib/tooltipped-content' import { RelativeTime } from '../relative-time' import classNames from 'classnames' +import { generateBranchContextMenuItems } from './branch-list-item-context-menu' interface IBranchListItemProps { /** The name of the branch */ @@ -74,30 +73,13 @@ export class BranchListItem extends React.Component< return } - const items: Array = [] - - if (onRenameBranch !== undefined) { - items.push({ - label: 'Rename…', - action: () => onRenameBranch(name), - enabled: isLocal, - }) - } - - items.push({ - label: __DARWIN__ ? 'Copy Branch Name' : 'Copy branch name', - action: () => clipboard.writeText(name), + const items = generateBranchContextMenuItems({ + name, + isLocal, + onRenameBranch, + onDeleteBranch, }) - items.push({ type: 'separator' }) - - if (onDeleteBranch !== undefined) { - items.push({ - label: 'Delete…', - action: () => onDeleteBranch(name), - }) - } - showContextualMenu(items) } diff --git a/app/src/ui/branches/branches-container.tsx b/app/src/ui/branches/branches-container.tsx index 0b6215c3eb..be94793be3 100644 --- a/app/src/ui/branches/branches-container.tsx +++ b/app/src/ui/branches/branches-container.tsx @@ -5,7 +5,7 @@ import { Repository, isRepositoryWithGitHubRepository, } from '../../models/repository' -import { Branch, BranchType } from '../../models/branch' +import { Branch } from '../../models/branch' import { BranchesTab } from '../../models/branches-tab' import { PopupType } from '../../models/popup' @@ -40,6 +40,8 @@ interface IBranchesContainerProps { readonly currentBranch: Branch | null readonly recentBranches: ReadonlyArray readonly pullRequests: ReadonlyArray + readonly onRenameBranch: (branchName: string) => void + readonly onDeleteBranch: (branchName: string) => void /** The pull request associated with the current branch. */ readonly currentPullRequest: PullRequest | null @@ -204,8 +206,8 @@ export class BranchesContainer extends React.Component< item, matches, this.props.currentBranch, - this.onRenameBranch, - this.onDeleteBranch, + this.props.onRenameBranch, + this.props.onDeleteBranch, this.onDropOntoBranch, this.onDropOntoCurrentBranch ) @@ -401,52 +403,6 @@ export class BranchesContainer extends React.Component< this.setState({ selectedPullRequest }) } - private getBranchWithName(branchName: string): Branch | undefined { - return this.props.allBranches.find(branch => branch.name === branchName) - } - - private onRenameBranch = (branchName: string) => { - const branch = this.getBranchWithName(branchName) - - if (branch === undefined) { - return - } - - this.props.dispatcher.showPopup({ - type: PopupType.RenameBranch, - repository: this.props.repository, - branch: branch, - }) - } - - private onDeleteBranch = async (branchName: string) => { - const branch = this.getBranchWithName(branchName) - - if (branch === undefined) { - return - } - - if (branch.type === BranchType.Remote) { - this.props.dispatcher.showPopup({ - type: PopupType.DeleteRemoteBranch, - repository: this.props.repository, - branch, - }) - return - } - - const aheadBehind = await this.props.dispatcher.getBranchAheadBehind( - this.props.repository, - branch - ) - this.props.dispatcher.showPopup({ - type: PopupType.DeleteBranch, - repository: this.props.repository, - branch, - existsOnRemote: aheadBehind !== null, - }) - } - /** * Method is to handle when something is dragged and dropped onto a branch * in the branch dropdown. diff --git a/app/src/ui/clone-repository/clone-repository.tsx b/app/src/ui/clone-repository/clone-repository.tsx index a8896f9295..d89646a093 100644 --- a/app/src/ui/clone-repository/clone-repository.tsx +++ b/app/src/ui/clone-repository/clone-repository.tsx @@ -187,6 +187,10 @@ export class CloneRepository extends React.Component< if (prevProps.selectedTab !== this.props.selectedTab) { this.validatePath() } + + if (prevProps.initialURL !== this.props.initialURL) { + this.updateUrl(this.props.initialURL || '') + } } public componentDidMount() { diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index dfa8a15813..9a959eb498 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -237,10 +237,18 @@ export class Dispatcher { repository: Repository, shas: ReadonlyArray, isContiguous: boolean - ): Promise { + ): void { return this.appStore._changeCommitSelection(repository, shas, isContiguous) } + /** Update the shas that should be highlighted */ + public updateShasToHighlight( + repository: Repository, + shasToHighlight: ReadonlyArray + ) { + this.appStore._updateShasToHighlight(repository, shasToHighlight) + } + /** * Change the selected changed file in the history view. * diff --git a/app/src/ui/history/commit-list.tsx b/app/src/ui/history/commit-list.tsx index 3943ed78ee..d176775bac 100644 --- a/app/src/ui/history/commit-list.tsx +++ b/app/src/ui/history/commit-list.tsx @@ -6,6 +6,7 @@ import { CommitListItem } from './commit-list-item' import { List } from '../lib/list' import { arrayEquals } from '../../lib/equality' import { DragData, DragType } from '../../models/drag-drop' +import classNames from 'classnames' const RowHeight = 50 @@ -23,13 +24,13 @@ interface ICommitListProps { readonly selectedSHAs: ReadonlyArray /** Whether or not commits in this list can be undone. */ - readonly canUndoCommits: boolean + readonly canUndoCommits?: boolean /** Whether or not commits in this list can be amended. */ - readonly canAmendCommits: boolean + readonly canAmendCommits?: boolean /** Whether or the user can reset to commits in this list. */ - readonly canResetToCommits: boolean + readonly canResetToCommits?: boolean /** The emoji lookup to render images inline */ readonly emoji: Map @@ -38,42 +39,42 @@ interface ICommitListProps { readonly localCommitSHAs: ReadonlyArray /** The message to display inside the list when no results are displayed */ - readonly emptyListMessage: JSX.Element | string + readonly emptyListMessage?: JSX.Element | string /** Callback which fires when a commit has been selected in the list */ - readonly onCommitsSelected: ( + readonly onCommitsSelected?: ( commits: ReadonlyArray, isContiguous: boolean ) => void /** Callback that fires when a scroll event has occurred */ - readonly onScroll: (start: number, end: number) => void + readonly onScroll?: (start: number, end: number) => void /** Callback to fire to undo a given commit in the current repository */ - readonly onUndoCommit: ((commit: Commit) => void) | undefined + readonly onUndoCommit?: (commit: Commit) => void /** Callback to fire to reset to a given commit in the current repository */ - readonly onResetToCommit: (commit: Commit) => void + readonly onResetToCommit?: (commit: Commit) => void /** Callback to fire to revert a given commit in the current repository */ - readonly onRevertCommit: ((commit: Commit) => void) | undefined + readonly onRevertCommit?: (commit: Commit) => void readonly onAmendCommit?: (commit: Commit, isLocalCommit: boolean) => void /** Callback to fire to open a given commit on GitHub */ - readonly onViewCommitOnGitHub: (sha: string) => void + readonly onViewCommitOnGitHub?: (sha: string) => void /** * Callback to fire to create a branch from a given commit in the current * repository */ - readonly onCreateBranch: (commit: CommitOneLine) => void + readonly onCreateBranch?: (commit: CommitOneLine) => void /** Callback to fire to open the dialog to create a new tag on the given commit */ - readonly onCreateTag: (targetCommitSha: string) => void + readonly onCreateTag?: (targetCommitSha: string) => void /** Callback to fire to delete an unpushed tag */ - readonly onDeleteTag: (tagName: string) => void + readonly onDeleteTag?: (tagName: string) => void /** * A handler called whenever the user drops commits on the list to be inserted. @@ -90,10 +91,10 @@ interface ICommitListProps { ) => void /** Callback to fire to cherry picking the commit */ - readonly onCherryPick: (commits: ReadonlyArray) => void + readonly onCherryPick?: (commits: ReadonlyArray) => void /** Callback to fire to squashing commits */ - readonly onSquash: ( + readonly onSquash?: ( toSquash: ReadonlyArray, squashOnto: Commit, lastRetainedCommitRef: string | null, @@ -113,25 +114,28 @@ interface ICommitListProps { readonly isLocalRepository: boolean /* Tags that haven't been pushed yet. This is used to show the unpushed indicator */ - readonly tagsToPush: ReadonlyArray | null + readonly tagsToPush?: ReadonlyArray /** Whether or not commits in this list can be reordered. */ - readonly reorderingEnabled: boolean + readonly reorderingEnabled?: boolean /** Whether a cherry pick is progress */ - readonly isCherryPickInProgress: boolean + readonly isCherryPickInProgress?: boolean /** Callback to render commit drag element */ - readonly onRenderCommitDragElement: ( + readonly onRenderCommitDragElement?: ( commit: Commit, selectedCommits: ReadonlyArray ) => void /** Callback to remove commit drag element */ - readonly onRemoveCommitDragElement: () => void + readonly onRemoveCommitDragElement?: () => void /** Whether squashing should be enabled on the commit list */ readonly disableSquashing?: boolean + + /** Shas that should be highlighted */ + readonly shasToHighlight?: ReadonlyArray } /** A component which displays the list of commits. */ @@ -163,7 +167,7 @@ export class CommitList extends React.Component { return null } - const tagsToPushSet = new Set(this.props.tagsToPush || []) + const tagsToPushSet = new Set(this.props.tagsToPush ?? []) const isLocal = this.props.localCommitSHAs.includes(commit.sha) const unpushedTags = commit.tags.filter(tagName => @@ -185,9 +189,11 @@ export class CommitList extends React.Component { key={commit.sha} gitHubRepository={this.props.gitHubRepository} isLocal={isLocal} - canBeUndone={this.props.canUndoCommits && isLocal && row === 0} - canBeAmended={this.props.canAmendCommits && row === 0} - canBeResetTo={this.props.canResetToCommits && isResettableCommit} + canBeUndone={this.props.canUndoCommits === true && isLocal && row === 0} + canBeAmended={this.props.canAmendCommits === true && row === 0} + canBeResetTo={ + this.props.canResetToCommits === true && isResettableCommit + } showUnpushedIndicator={showUnpushedIndicator} unpushedIndicatorTitle={this.getUnpushedIndicatorTitle( isLocal, @@ -233,7 +239,7 @@ export class CommitList extends React.Component { const indexes = [...toSquash, squashOnto].map(v => this.props.commitSHAs.findIndex(sha => sha === v.sha) ) - this.props.onSquash( + this.props.onSquash?.( toSquash, squashOnto, this.getLastRetainedCommitRef(indexes), @@ -242,7 +248,7 @@ export class CommitList extends React.Component { } private onRenderCommitDragElement = (commit: Commit) => { - this.props.onRenderCommitDragElement( + this.props.onRenderCommitDragElement?.( commit, this.lookupCommits(this.props.selectedSHAs) ) @@ -274,7 +280,7 @@ export class CommitList extends React.Component { const sorted = [...rows].sort((a, b) => b - a) const selectedShas = sorted.map(r => this.props.commitSHAs[r]) const selectedCommits = this.lookupCommits(selectedShas) - this.props.onCommitsSelected(selectedCommits, this.isContiguous(sorted)) + this.props.onCommitsSelected?.(selectedCommits, this.isContiguous(sorted)) } /** @@ -308,7 +314,7 @@ export class CommitList extends React.Component { const sha = this.props.commitSHAs[row] const commit = this.props.commitLookup.get(sha) if (commit) { - this.props.onCommitsSelected([commit], true) + this.props.onCommitsSelected?.([commit], true) } } @@ -333,12 +339,10 @@ export class CommitList extends React.Component { const numberOfRows = Math.ceil(clientHeight / RowHeight) const top = Math.floor(scrollTop / RowHeight) const bottom = top + numberOfRows - this.props.onScroll(top, bottom) + this.props.onScroll?.(top, bottom) // Pass new scroll value so the scroll position will be remembered (if the callback has been supplied). - if (this.props.onCompareListScrolled != null) { - this.props.onCompareListScrolled(scrollTop) - } + this.props.onCompareListScrolled?.(scrollTop) } private rowForSHA(sha_: string | null): number { @@ -350,19 +354,47 @@ export class CommitList extends React.Component { return this.props.commitSHAs.findIndex(s => s === sha) } + private getRowCustomClassMap = () => { + const { commitSHAs, shasToHighlight } = this.props + if (shasToHighlight === undefined || shasToHighlight.length === 0) { + return undefined + } + + const rowsForShasNotInDiff = commitSHAs + .filter(sha => shasToHighlight.includes(sha)) + .map(sha => this.rowForSHA(sha)) + + if (rowsForShasNotInDiff.length === 0) { + return undefined + } + + const rowClassMap = new Map>() + rowClassMap.set('highlighted', rowsForShasNotInDiff) + return rowClassMap + } + public render() { - if (this.props.commitSHAs.length === 0) { + const { commitSHAs, selectedSHAs, shasToHighlight, emptyListMessage } = + this.props + if (commitSHAs.length === 0) { return ( -
{this.props.emptyListMessage}
+
+ {emptyListMessage ?? 'No commits to list'} +
) } + const classes = classNames({ + 'has-highlighted-commits': + shasToHighlight !== undefined && shasToHighlight.length > 0, + }) + return ( -
+
this.rowForSHA(sha))} + selectedRows={selectedSHAs.map(sha => this.rowForSHA(sha))} rowRenderer={this.renderCommit} onDropDataInsertion={this.onDropDataInsertion} onSelectionChanged={this.onSelectionChanged} @@ -370,15 +402,17 @@ export class CommitList extends React.Component { selectionMode="multi" onScroll={this.onScroll} insertionDragType={ - this.props.reorderingEnabled ? DragType.Commit : undefined + this.props.reorderingEnabled === true ? DragType.Commit : undefined } invalidationProps={{ commits: this.props.commitSHAs, localCommitSHAs: this.props.localCommitSHAs, commitLookupHash: this.commitsHash(this.getVisibleCommits()), tagsToPush: this.props.tagsToPush, + shasToHighlight: this.props.shasToHighlight, }} setScrollTop={this.props.compareListScrollTop} + rowCustomClassNameMap={this.getRowCustomClassMap()} />
) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index f16c83a66a..c8853a9086 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -19,10 +19,13 @@ import { clipboard } from 'electron' import { TooltipDirection } from '../lib/tooltip' import { AppFileStatusKind } from '../../models/status' import _ from 'lodash' +import { LinkButton } from '../lib/link-button' +import { UnreachableCommitsTab } from './unreachable-commits-dialog' interface ICommitSummaryProps { readonly repository: Repository - readonly commits: ReadonlyArray + readonly selectedCommits: ReadonlyArray + readonly shasInDiff: ReadonlyArray readonly changesetData: IChangesetData readonly emoji: Map @@ -52,6 +55,12 @@ interface ICommitSummaryProps { /** Called when the user opens the diff options popover */ readonly onDiffOptionsOpened: () => void + + /** Called to highlight certain shas in the history */ + readonly onHighlightShas: (shasToHighlight: ReadonlyArray) => void + + /** Called to show unreachable commits dialog */ + readonly showUnreachableCommits: (tab: UnreachableCommitsTab) => void } interface ICommitSummaryState { @@ -62,6 +71,11 @@ interface ICommitSummaryState { */ readonly summary: ReadonlyArray + /** + * Whether the commit summary was empty. + */ + readonly hasEmptySummary: boolean + /** * The commit message body, i.e. anything after the first line of text in the * commit message. Note that this may differ from the body property in the @@ -99,36 +113,49 @@ function createState( isOverflowed: boolean, props: ICommitSummaryProps ): ICommitSummaryState { - const { emoji, repository, commits } = props + const { emoji, repository, selectedCommits } = props const tokenizer = new Tokenizer(emoji, repository) - const plainTextBody = - commits.length > 1 - ? commits - .map( - c => - `${c.shortSha} - ${c.summary}${ - c.body.trim() !== '' ? `\n${c.body}` : '' - }` - ) - .join('\n\n') - : commits[0].body - const { summary, body } = wrapRichTextCommitMessage( - commits[0].summary, - plainTextBody, + getCommitSummary(selectedCommits), + selectedCommits[0].body, tokenizer ) - const allAvatarUsers = commits.flatMap(c => + const hasEmptySummary = + selectedCommits.length === 1 && selectedCommits[0].summary.length === 0 + + const allAvatarUsers = selectedCommits.flatMap(c => getAvatarUsersForCommit(repository.gitHubRepository, c) ) + const avatarUsers = _.uniqWith( allAvatarUsers, (a, b) => a.email === b.email && a.name === b.name ) - return { isOverflowed, summary, body, avatarUsers } + return { isOverflowed, summary, body, avatarUsers, hasEmptySummary } +} + +function getCommitSummary(selectedCommits: ReadonlyArray) { + return selectedCommits[0].summary.length === 0 + ? 'Empty commit message' + : selectedCommits[0].summary +} + +function getCountCommitsNotInDiff( + selectedCommits: ReadonlyArray, + shasInDiff: ReadonlyArray +) { + if (selectedCommits.length === 1) { + return 0 + } + + const excludedCommits = selectedCommits.filter( + ({ sha }) => !shasInDiff.includes(sha) + ) + + return excludedCommits.length } /** @@ -260,9 +287,9 @@ export class CommitSummary extends React.Component< public componentWillUpdate(nextProps: ICommitSummaryProps) { if ( - nextProps.commits.length !== this.props.commits.length || - !nextProps.commits.every((nextCommit, i) => - messageEquals(nextCommit, this.props.commits[i]) + nextProps.selectedCommits.length !== this.props.selectedCommits.length || + !nextProps.selectedCommits.every((nextCommit, i) => + messageEquals(nextCommit, this.props.selectedCommits[i]) ) ) { this.setState(createState(false, nextProps)) @@ -316,17 +343,154 @@ export class CommitSummary extends React.Component< } private getShaRef = (useShortSha?: boolean) => { - const { commits } = this.props - const oldest = useShortSha ? commits[0].shortSha : commits[0].sha + const { selectedCommits } = this.props + return useShortSha ? selectedCommits[0].shortSha : selectedCommits[0].sha + } - if (commits.length === 1) { - return oldest + private onHighlightShasInDiff = () => { + this.props.onHighlightShas(this.props.shasInDiff) + } + + private onHighlightShasNotInDiff = () => { + const { onHighlightShas, selectedCommits, shasInDiff } = this.props + onHighlightShas( + selectedCommits.filter(c => !shasInDiff.includes(c.sha)).map(c => c.sha) + ) + } + + private onRemoveHighlightOfShas = () => { + this.props.onHighlightShas([]) + } + + private showUnreachableCommits = () => { + this.props.showUnreachableCommits(UnreachableCommitsTab.Unreachable) + } + + private showReachableCommits = () => { + this.props.showUnreachableCommits(UnreachableCommitsTab.Reachable) + } + + private renderCommitsNotReachable = () => { + const { selectedCommits, shasInDiff } = this.props + if (selectedCommits.length === 1) { + return } - const latestCommit = commits.at(-1) - const latest = useShortSha ? latestCommit?.shortSha : latestCommit?.sha + const excludedCommitsCount = getCountCommitsNotInDiff( + selectedCommits, + shasInDiff + ) - return `${oldest}^..${latest}` + if (excludedCommitsCount === 0) { + return + } + + const commitsPluralized = excludedCommitsCount > 1 ? 'commits' : 'commit' + + return ( +
+ + + {excludedCommitsCount} unreachable {commitsPluralized} + {' '} + not included. +
+ ) + } + + private renderAuthors = () => { + const { selectedCommits, repository } = this.props + const { avatarUsers } = this.state + if (selectedCommits.length > 1) { + return + } + + return ( +
  • + + +
    • + ) + } + + private renderCommitRef = () => { + const { selectedCommits } = this.props + if (selectedCommits.length > 1) { + return + } + + return ( +
  • + + + {this.getShaRef(true)} + +
    • + ) + } + + private renderSummary = () => { + const { selectedCommits, shasInDiff } = this.props + const { summary, hasEmptySummary } = this.state + const summaryClassNames = classNames('commit-summary-title', { + 'empty-summary': hasEmptySummary, + }) + + if (selectedCommits.length === 1) { + return ( + + ) + } + + const commitsNotInDiff = getCountCommitsNotInDiff( + selectedCommits, + shasInDiff + ) + const numInDiff = selectedCommits.length - commitsNotInDiff + const commitsPluralized = numInDiff > 1 ? 'commits' : 'commit' + return ( +
    + Showing changes from{' '} + {commitsNotInDiff > 0 ? ( + + {numInDiff} {commitsPluralized} + + ) : ( + <> + {' '} + {numInDiff} {commitsPluralized} + + )} +
    + ) } public render() { @@ -337,55 +501,13 @@ export class CommitSummary extends React.Component< 'hide-description-border': this.props.hideDescriptionBorder, }) - const hasEmptySummary = this.state.summary.length === 0 - const commitSummary = hasEmptySummary - ? 'Empty commit message' - : this.props.commits.length > 1 - ? `Viewing the diff of ${this.props.commits.length} commits` - : this.state.summary - - const summaryClassNames = classNames('commit-summary-title', { - 'empty-summary': hasEmptySummary, - }) - return (
    - - + {this.renderSummary()}
      -
    • - - -
      • - -
    • - - - {this.getShaRef(true)} - -
      • - + {this.renderAuthors()} + {this.renderCommitRef()} {this.renderChangedFilesDescription()} {this.renderLinesChanged()} {this.renderTags()} @@ -411,6 +533,7 @@ export class CommitSummary extends React.Component<
    {this.renderDescription()} + {this.renderCommitsNotReachable()}
    ) } @@ -528,10 +651,15 @@ export class CommitSummary extends React.Component< } private renderTags() { - const tags = this.props.commits.flatMap(c => c.tags) || [] + const { selectedCommits } = this.props + if (selectedCommits.length > 1) { + return + } + + const tags = selectedCommits[0].tags if (tags.length === 0) { - return null + return } return ( diff --git a/app/src/ui/history/compare.tsx b/app/src/ui/history/compare.tsx index 7d74d46edd..4a036d9af3 100644 --- a/app/src/ui/history/compare.tsx +++ b/app/src/ui/history/compare.tsx @@ -55,6 +55,7 @@ interface ICompareSidebarProps { readonly tagsToPush: ReadonlyArray | null readonly aheadBehindStore: AheadBehindStore readonly isCherryPickInProgress: boolean + readonly shasToHighlight: ReadonlyArray } interface ICompareSidebarState { @@ -230,6 +231,7 @@ export class CompareSidebar extends React.Component< commitLookup={this.props.commitLookup} commitSHAs={commitSHAs} selectedSHAs={this.props.selectedCommitShas} + shasToHighlight={this.props.shasToHighlight} localCommitSHAs={this.props.localCommitSHAs} canResetToCommits={formState.kind === HistoryTabMode.History} canUndoCommits={formState.kind === HistoryTabMode.History} @@ -258,7 +260,7 @@ export class CompareSidebar extends React.Component< emptyListMessage={emptyListMessage} onCompareListScrolled={this.props.onCompareListScrolled} compareListScrollTop={this.props.compareListScrollTop} - tagsToPush={this.props.tagsToPush} + tagsToPush={this.props.tagsToPush ?? []} isCherryPickInProgress={this.props.isCherryPickInProgress} onRenderCommitDragElement={this.onRenderCommitDragElement} onRemoveCommitDragElement={this.onRemoveCommitDragElement} diff --git a/app/src/ui/history/index.ts b/app/src/ui/history/index.ts index 7603ced06a..f4fabb6e82 100644 --- a/app/src/ui/history/index.ts +++ b/app/src/ui/history/index.ts @@ -1,2 +1,2 @@ -export { SelectedCommits } from './selected-commit' +export { SelectedCommits } from './selected-commits' export { CompareSidebar } from './compare' diff --git a/app/src/ui/history/selected-commit.tsx b/app/src/ui/history/selected-commits.tsx similarity index 94% rename from app/src/ui/history/selected-commit.tsx rename to app/src/ui/history/selected-commits.tsx index 3b31b63dff..150f7ff9b2 100644 --- a/app/src/ui/history/selected-commit.tsx +++ b/app/src/ui/history/selected-commits.tsx @@ -35,6 +35,8 @@ import { IConstrainedValue } from '../../lib/app-state' import { clamp } from '../../lib/clamp' import { pathExists } from '../lib/path-exists' import { enableMultiCommitDiffs } from '../../lib/feature-flag' +import { PopupType } from '../../models/popup' +import { UnreachableCommitsTab } from './unreachable-commits-dialog' interface ISelectedCommitsProps { readonly repository: Repository @@ -42,6 +44,7 @@ interface ISelectedCommitsProps { readonly dispatcher: Dispatcher readonly emoji: Map readonly selectedCommits: ReadonlyArray + readonly shasInDiff: ReadonlyArray readonly localCommitSHAs: ReadonlyArray readonly changesetData: IChangesetData readonly selectedFile: CommittedFileChange | null @@ -166,7 +169,8 @@ export class SelectedCommits extends React.Component< private renderCommitSummary(commits: ReadonlyArray) { return ( ) } + private showUnreachableCommits = (selectedTab: UnreachableCommitsTab) => { + this.props.dispatcher.showPopup({ + type: PopupType.UnreachableCommits, + selectedTab, + }) + } + + private onHighlightShas = (shasToHighlight: ReadonlyArray) => { + this.props.dispatcher.updateShasToHighlight( + this.props.repository, + shasToHighlight + ) + } + private onExpandChanged = (isExpanded: boolean) => { this.setState({ isExpanded }) } diff --git a/app/src/ui/history/unreachable-commits-dialog.tsx b/app/src/ui/history/unreachable-commits-dialog.tsx new file mode 100644 index 0000000000..7919e09f11 --- /dev/null +++ b/app/src/ui/history/unreachable-commits-dialog.tsx @@ -0,0 +1,149 @@ +import * as React from 'react' +import { Dialog, DialogFooter } from '../dialog' +import { TabBar } from '../tab-bar' +import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' +import { Commit } from '../../models/commit' +import { CommitList } from './commit-list' + +export enum UnreachableCommitsTab { + Unreachable, + Reachable, +} + +interface IUnreachableCommitsDialogProps { + /** The shas of the currently selected commits */ + readonly selectedShas: ReadonlyArray + + /** The shas of the commits showed in the diff */ + readonly shasInDiff: ReadonlyArray + + /** The commits loaded, keyed by their full SHA. */ + readonly commitLookup: Map + + /** Used to set the selected tab. */ + readonly selectedTab: UnreachableCommitsTab + + /** The emoji lookup to render images inline */ + readonly emoji: Map + + /** Called to dismiss the */ + readonly onDismissed: () => void +} + +interface IUnreachableCommitsDialogState { + /** The currently select tab. */ + readonly selectedTab: UnreachableCommitsTab +} + +/** The component for for viewing the unreachable commits in the current diff a repository. */ +export class UnreachableCommitsDialog extends React.Component< + IUnreachableCommitsDialogProps, + IUnreachableCommitsDialogState +> { + public constructor(props: IUnreachableCommitsDialogProps) { + super(props) + + this.state = { + selectedTab: props.selectedTab, + } + } + + public componentWillUpdate(nextProps: IUnreachableCommitsDialogProps) { + const currentSelectedTab = this.props.selectedTab + const selectedTab = nextProps.selectedTab + + if (currentSelectedTab !== selectedTab) { + this.setState({ selectedTab }) + } + } + + private onTabClicked = (selectedTab: UnreachableCommitsTab) => { + this.setState({ selectedTab }) + } + + private getShasToDisplay = () => { + const { selectedTab } = this.state + const { shasInDiff, selectedShas } = this.props + if (selectedTab === UnreachableCommitsTab.Reachable) { + return shasInDiff + } + + return selectedShas.filter(sha => !shasInDiff.includes(sha)) + } + + private renderTabs() { + return ( + + Unreachable + Reachable + + ) + } + + private renderActiveTab() { + const { commitLookup, emoji } = this.props + + return ( + <> + {this.renderUnreachableCommitsMessage()} +
    + +
    + + ) + } + + private renderFooter() { + return ( + + + + ) + } + + private renderUnreachableCommitsMessage = () => { + const count = this.getShasToDisplay().length + const commitsPluralized = count > 1 ? 'commits' : 'commit' + const pronounPluralized = count > 1 ? `they're` : `it's` + return ( +
    + You will{' '} + {this.state.selectedTab === UnreachableCommitsTab.Unreachable + ? 'not' + : ''}{' '} + see changes from the following {commitsPluralized} because{' '} + {pronounPluralized}{' '} + {this.state.selectedTab === UnreachableCommitsTab.Unreachable + ? 'not' + : ''}{' '} + in the ancestry path of the most recent commit in your selection. +
    + ) + } + + public render() { + return ( + + {this.renderTabs()} + {this.renderActiveTab()} + {this.renderFooter()} + + ) + } +} diff --git a/app/src/ui/index.tsx b/app/src/ui/index.tsx index 997818a83f..693154216c 100644 --- a/app/src/ui/index.tsx +++ b/app/src/ui/index.tsx @@ -202,6 +202,13 @@ process.on( } ) +// HACK: this is a workaround for a known crash in the Dev Tools on Electron 19 +// See https://github.com/electron/electron/issues/34350 +if (__DEV__) { + window.onerror = e => + e === 'Uncaught EvalError: Possible side-effect in debug-evaluate' +} + /** * Chromium won't crash on an unhandled rejection (similar to how it won't crash * on an unhandled error). We've taken the approach that unhandled errors should diff --git a/app/src/ui/lib/button.tsx b/app/src/ui/lib/button.tsx index 4e7f21409f..3a79753c1b 100644 --- a/app/src/ui/lib/button.tsx +++ b/app/src/ui/lib/button.tsx @@ -8,10 +8,18 @@ export interface IButtonProps { * A callback which is invoked when the button is clicked * using a pointer device or keyboard. The source event is * passed along and can be used to prevent the default action - * or stop the even from bubbling. + * or stop the event from bubbling. */ readonly onClick?: (event: React.MouseEvent) => void + /** + * A callback which is invoked when the button's context menu + * is activated using a pointer device or keyboard. The source + * event is passed along and can be used to prevent the default + * action or stop the event from bubbling. + */ + readonly onContextMenu?: (event: React.MouseEvent) => void + /** * A function that's called when the user moves over the button with * a pointer device. @@ -118,6 +126,7 @@ export class Button extends React.Component {