From b21de4e597c20a8f681153a5d000c29753a744c6 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 17 May 2022 14:13:02 -0400 Subject: [PATCH 01/84] Create a markdown emitter for parsed markdown --- .../lib/markdown-filters/markdown-filter.ts | 34 +++++++++++++++--- app/src/ui/lib/sandboxed-markdown.tsx | 36 +++++++++++++------ 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/app/src/lib/markdown-filters/markdown-filter.ts b/app/src/lib/markdown-filters/markdown-filter.ts index d954d7104f..0c8af1dcaf 100644 --- a/app/src/lib/markdown-filters/markdown-filter.ts +++ b/app/src/lib/markdown-filters/markdown-filter.ts @@ -1,4 +1,5 @@ import DOMPurify from 'dompurify' +import { Disposable, Emitter } from 'event-kit' import { marked } from 'marked' import { GitHubRepository } from '../../models/github-repository' import { @@ -13,6 +14,24 @@ interface ICustomMarkdownFilterOptions { markdownContext?: MarkdownContext } +export class MarkdownEmitter extends Emitter { + public constructor(private markdown: null | string = null) { + super() + } + + public onMarkdownUpdated(handler: (value: string) => void): Disposable { + if (this.markdown !== null) { + handler(this.markdown) + } + return super.on('markdown', handler) + } + + public emit(value: string): void { + this.markdown = value + super.emit('markdown', value) + } +} + /** * Takes string of markdown and runs it through the MarkedJs parser with github * flavored flags enabled followed by running that through domPurify, and lastly @@ -22,7 +41,7 @@ interface ICustomMarkdownFilterOptions { export async function parseMarkdown( markdown: string, customMarkdownOptions?: ICustomMarkdownFilterOptions -) { +): Promise { const parsedMarkdown = marked(markdown, { // https://marked.js.org/using_advanced If true, use approved GitHub // Flavored Markdown (GFM) specification. @@ -33,11 +52,16 @@ export async function parseMarkdown( breaks: true, }) - const sanitizedHTML = DOMPurify.sanitize(parsedMarkdown) + const sanitizedMarkdown = DOMPurify.sanitize(parsedMarkdown) + const filteredMarkdown = + customMarkdownOptions !== undefined + ? await applyCustomMarkdownFilters( + sanitizedMarkdown, + customMarkdownOptions + ) + : sanitizedMarkdown - return customMarkdownOptions !== undefined - ? await applyCustomMarkdownFilters(sanitizedHTML, customMarkdownOptions) - : sanitizedHTML + return new MarkdownEmitter(filteredMarkdown) } /** diff --git a/app/src/ui/lib/sandboxed-markdown.tsx b/app/src/ui/lib/sandboxed-markdown.tsx index 79d90ed812..cafceb480d 100644 --- a/app/src/ui/lib/sandboxed-markdown.tsx +++ b/app/src/ui/lib/sandboxed-markdown.tsx @@ -7,11 +7,14 @@ import { Tooltip } from './tooltip' import { createObservableRef } from './observable-ref' import { getObjectId } from './object-id' import { debounce } from 'lodash' -import { parseMarkdown } from '../../lib/markdown-filters/markdown-filter' +import { + MarkdownEmitter, + parseMarkdown, +} from '../../lib/markdown-filters/markdown-filter' interface ISandboxedMarkdownProps { /** A string of unparsed markdown to display */ - readonly markdown: string + readonly markdown: string | MarkdownEmitter /** Whether the markdown was pre-parsed - assumed false */ readonly isParsed?: boolean @@ -58,6 +61,7 @@ export class SandboxedMarkdown extends React.PureComponent< private frameRef: HTMLIFrameElement | null = null private frameContainingDivRef: HTMLDivElement | null = null private contentDivRef: HTMLDivElement | null = null + private markdownEmitter?: MarkdownEmitter /** * Resize observer used for tracking height changes in the markdown @@ -105,8 +109,22 @@ export class SandboxedMarkdown extends React.PureComponent< this.frameContainingDivRef = frameContainingDivRef } + private initializeMarkdownEmitter = async () => { + if (this.markdownEmitter !== undefined) { + this.markdownEmitter.dispose() + } + this.markdownEmitter = + typeof this.props.markdown !== 'string' + ? this.props.markdown + : await parseMarkdown(this.props.markdown) + + this.markdownEmitter.onMarkdownUpdated((markdown: string) => { + this.mountIframeContents(markdown) + }) + } + public async componentDidMount() { - this.mountIframeContents() + await this.initializeMarkdownEmitter() if (this.frameRef !== null) { this.setupFrameLoadListeners(this.frameRef) @@ -120,11 +138,12 @@ export class SandboxedMarkdown extends React.PureComponent< public async componentDidUpdate(prevProps: ISandboxedMarkdownProps) { // rerender iframe contents if provided markdown changes if (prevProps.markdown !== this.props.markdown) { - this.mountIframeContents() + this.initializeMarkdownEmitter() } } public componentWillUnmount() { + this.markdownEmitter?.dispose() this.resizeObserver.disconnect() document.removeEventListener('scroll', this.onDocumentScroll) } @@ -288,18 +307,13 @@ export class SandboxedMarkdown extends React.PureComponent< /** * Populates the mounted iframe with HTML generated from the provided markdown */ - private async mountIframeContents() { + private async mountIframeContents(markdown: string) { if (this.frameRef === null) { return } const styleSheet = await this.getInlineStyleSheet() - const filteredHTML = - this.props.isParsed === true - ? this.props.markdown - : await parseMarkdown(this.props.markdown) - const src = ` @@ -308,7 +322,7 @@ export class SandboxedMarkdown extends React.PureComponent<
- ${filteredHTML} + ${markdown}
From 9ad3fd384fd887ec6a3e234410731e0ddb07e355 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 17 May 2022 14:36:49 -0400 Subject: [PATCH 02/84] Remove unused property --- app/src/ui/lib/sandboxed-markdown.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/src/ui/lib/sandboxed-markdown.tsx b/app/src/ui/lib/sandboxed-markdown.tsx index cafceb480d..65d8575ffe 100644 --- a/app/src/ui/lib/sandboxed-markdown.tsx +++ b/app/src/ui/lib/sandboxed-markdown.tsx @@ -16,9 +16,6 @@ interface ISandboxedMarkdownProps { /** A string of unparsed markdown to display */ readonly markdown: string | MarkdownEmitter - /** Whether the markdown was pre-parsed - assumed false */ - readonly isParsed?: boolean - /** The baseHref of the markdown content for when the markdown has relative links */ readonly baseHref?: string From a5bb19f75d6216087fb7d4826b58a96b76f6e79c Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 17 May 2022 14:52:19 -0400 Subject: [PATCH 03/84] Put the custom markdown filters back --- app/src/ui/lib/sandboxed-markdown.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/src/ui/lib/sandboxed-markdown.tsx b/app/src/ui/lib/sandboxed-markdown.tsx index 65d8575ffe..4f270da95c 100644 --- a/app/src/ui/lib/sandboxed-markdown.tsx +++ b/app/src/ui/lib/sandboxed-markdown.tsx @@ -110,10 +110,15 @@ export class SandboxedMarkdown extends React.PureComponent< if (this.markdownEmitter !== undefined) { this.markdownEmitter.dispose() } + const { emoji, repository, markdownContext } = this.props this.markdownEmitter = typeof this.props.markdown !== 'string' ? this.props.markdown - : await parseMarkdown(this.props.markdown) + : parseMarkdown(this.props.markdown, { + emoji, + repository, + markdownContext, + }) this.markdownEmitter.onMarkdownUpdated((markdown: string) => { this.mountIframeContents(markdown) From cc7911087578ed9f80dff45d78753d89653e86bc Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 17 May 2022 15:29:57 -0400 Subject: [PATCH 04/84] Make custom markdown filter use markdown emitter --- .../lib/markdown-filters/markdown-filter.ts | 28 ++++++++++--------- app/src/lib/markdown-filters/node-filter.ts | 19 +++++++++---- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/app/src/lib/markdown-filters/markdown-filter.ts b/app/src/lib/markdown-filters/markdown-filter.ts index 0c8af1dcaf..9820ade121 100644 --- a/app/src/lib/markdown-filters/markdown-filter.ts +++ b/app/src/lib/markdown-filters/markdown-filter.ts @@ -30,6 +30,10 @@ export class MarkdownEmitter extends Emitter { this.markdown = value super.emit('markdown', value) } + + public get latestMarkdown() { + return this.markdown + } } /** @@ -38,10 +42,10 @@ export class MarkdownEmitter extends Emitter { * if custom markdown options are provided, it applies the custom markdown * filters. */ -export async function parseMarkdown( +export function parseMarkdown( markdown: string, customMarkdownOptions?: ICustomMarkdownFilterOptions -): Promise { +): MarkdownEmitter { const parsedMarkdown = marked(markdown, { // https://marked.js.org/using_advanced If true, use approved GitHub // Flavored Markdown (GFM) specification. @@ -53,15 +57,13 @@ export async function parseMarkdown( }) const sanitizedMarkdown = DOMPurify.sanitize(parsedMarkdown) - const filteredMarkdown = - customMarkdownOptions !== undefined - ? await applyCustomMarkdownFilters( - sanitizedMarkdown, - customMarkdownOptions - ) - : sanitizedMarkdown + const markdownEmitter = new MarkdownEmitter(sanitizedMarkdown) - return new MarkdownEmitter(filteredMarkdown) + if (customMarkdownOptions !== undefined) { + applyCustomMarkdownFilters(markdownEmitter, customMarkdownOptions) + } + + return markdownEmitter } /** @@ -71,13 +73,13 @@ export async function parseMarkdown( * mentions, etc. */ function applyCustomMarkdownFilters( - parsedMarkdown: string, + markdownEmitter: MarkdownEmitter, options: ICustomMarkdownFilterOptions -): Promise { +): void { const nodeFilters = buildCustomMarkDownNodeFilterPipe( options.emoji, options.repository, options.markdownContext ) - return applyNodeFilters(nodeFilters, parsedMarkdown) + applyNodeFilters(nodeFilters, markdownEmitter) } diff --git a/app/src/lib/markdown-filters/node-filter.ts b/app/src/lib/markdown-filters/node-filter.ts index 4870aa39af..60991a90ba 100644 --- a/app/src/lib/markdown-filters/node-filter.ts +++ b/app/src/lib/markdown-filters/node-filter.ts @@ -13,6 +13,7 @@ import { isIssueClosingContext, } from './close-keyword-filter' import { CommitMentionLinkFilter } from './commit-mention-link-filter' +import { MarkdownEmitter } from './markdown-filter' export interface INodeFilter { /** @@ -47,7 +48,7 @@ export interface INodeFilter { export const buildCustomMarkDownNodeFilterPipe = memoizeOne( ( emoji: Map, - repository?: GitHubRepository, + repository?: GitHubRepository | undefined, markdownContext?: MarkdownContext ): ReadonlyArray => { const filterPipe: Array = [] @@ -104,15 +105,21 @@ export const buildCustomMarkDownNodeFilterPipe = memoizeOne( */ export async function applyNodeFilters( nodeFilters: ReadonlyArray, - parsedMarkdown: string -): Promise { - const mdDoc = new DOMParser().parseFromString(parsedMarkdown, 'text/html') + markdownEmitter: MarkdownEmitter +): Promise { + if (markdownEmitter.latestMarkdown === null) { + return + } + + const mdDoc = new DOMParser().parseFromString( + markdownEmitter.latestMarkdown, + 'text/html' + ) for (const nodeFilter of nodeFilters) { await applyNodeFilter(nodeFilter, mdDoc) + markdownEmitter.emit(mdDoc.documentElement.innerHTML) } - - return mdDoc.documentElement.innerHTML } /** From 2e8f09244658e95529f044ba01b88a6470d14ef6 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 17 May 2022 15:30:32 -0400 Subject: [PATCH 05/84] Debounce markdown updates --- app/src/ui/lib/sandboxed-markdown.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/app/src/ui/lib/sandboxed-markdown.tsx b/app/src/ui/lib/sandboxed-markdown.tsx index 4f270da95c..9b02638313 100644 --- a/app/src/ui/lib/sandboxed-markdown.tsx +++ b/app/src/ui/lib/sandboxed-markdown.tsx @@ -73,6 +73,18 @@ export class SandboxedMarkdown extends React.PureComponent< }) }, 100) + /** + * We debounce the markdown updating because it is updated on each custom + * markdown filter. Leading is true so that users will at a minimum see the + * markdown parsed by markedjs while the custom filters are being applied. + * (So instead of being updated, 10+ times it is updated 1 or 2 times.) + */ + private onMarkdownUpdated = debounce( + markdown => this.mountIframeContents(markdown), + 10, + { leading: true } + ) + public constructor(props: ISandboxedMarkdownProps) { super(props) @@ -121,7 +133,7 @@ export class SandboxedMarkdown extends React.PureComponent< }) this.markdownEmitter.onMarkdownUpdated((markdown: string) => { - this.mountIframeContents(markdown) + this.onMarkdownUpdated(markdown) }) } From 6f0a3384f804d38075c9e25f73151421d7af4749 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 17 May 2022 15:53:34 -0400 Subject: [PATCH 06/84] Tidying --- .../lib/markdown-filters/markdown-filter.ts | 37 +++++++++++-------- app/src/lib/markdown-filters/node-filter.ts | 17 +++++---- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/app/src/lib/markdown-filters/markdown-filter.ts b/app/src/lib/markdown-filters/markdown-filter.ts index 9820ade121..0151622616 100644 --- a/app/src/lib/markdown-filters/markdown-filter.ts +++ b/app/src/lib/markdown-filters/markdown-filter.ts @@ -1,19 +1,16 @@ import DOMPurify from 'dompurify' import { Disposable, Emitter } from 'event-kit' import { marked } from 'marked' -import { GitHubRepository } from '../../models/github-repository' import { applyNodeFilters, buildCustomMarkDownNodeFilterPipe, - MarkdownContext, + ICustomMarkdownFilterOptions, } from './node-filter' -interface ICustomMarkdownFilterOptions { - emoji: Map - repository?: GitHubRepository - markdownContext?: MarkdownContext -} - +/** + * The MarkdownEmitter extends the Emitter functionality to be able to keep + * track of the last emitted value and return it upon subscription. + */ export class MarkdownEmitter extends Emitter { public constructor(private markdown: null | string = null) { super() @@ -38,9 +35,21 @@ export class MarkdownEmitter extends Emitter { /** * Takes string of markdown and runs it through the MarkedJs parser with github - * flavored flags enabled followed by running that through domPurify, and lastly - * if custom markdown options are provided, it applies the custom markdown + * flavored flags followed by sanitization with domPurify. + * + * If custom markdown options are provided, it applies the custom markdown * filters. + * + * Rely `repository` custom markdown option: + * - TeamMentionFilter + * - MentionFilter + * - CommitMentionFilter + * - CommitMentionLinkFilter + * + * Rely `markdownContext` custom markdown option: + * - IssueMentionFilter + * - IssueLinkFilter + * - CloseKeyWordFilter */ export function parseMarkdown( markdown: string, @@ -70,16 +79,12 @@ export function parseMarkdown( * Applies custom markdown filters to parsed markdown html. This is done * through converting the markdown html into a DOM document and then * traversing the nodes to apply custom filters such as emoji, issue, username - * mentions, etc. + * mentions, etc. (Expects a markdownEmitter with an initial markdown value) */ function applyCustomMarkdownFilters( markdownEmitter: MarkdownEmitter, options: ICustomMarkdownFilterOptions ): void { - const nodeFilters = buildCustomMarkDownNodeFilterPipe( - options.emoji, - options.repository, - options.markdownContext - ) + const nodeFilters = buildCustomMarkDownNodeFilterPipe(options) applyNodeFilters(nodeFilters, markdownEmitter) } diff --git a/app/src/lib/markdown-filters/node-filter.ts b/app/src/lib/markdown-filters/node-filter.ts index 60991a90ba..cd8264ac0c 100644 --- a/app/src/lib/markdown-filters/node-filter.ts +++ b/app/src/lib/markdown-filters/node-filter.ts @@ -1,5 +1,4 @@ import memoizeOne from 'memoize-one' -import { GitHubRepository } from '../../models/github-repository' import { EmojiFilter } from './emoji-filter' import { IssueLinkFilter } from './issue-link-filter' import { IssueMentionFilter } from './issue-mention-filter' @@ -14,6 +13,7 @@ import { } from './close-keyword-filter' import { CommitMentionLinkFilter } from './commit-mention-link-filter' import { MarkdownEmitter } from './markdown-filter' +import { GitHubRepository } from '../../models/github-repository' export interface INodeFilter { /** @@ -38,19 +38,20 @@ export interface INodeFilter { filter(node: Node): Promise | null> } +export interface ICustomMarkdownFilterOptions { + emoji: Map + repository?: GitHubRepository + markdownContext?: MarkdownContext +} + /** * Builds an array of node filters to apply to markdown html. Referring to it as pipe * because they will be applied in the order they are entered in the returned * array. This is important as some filters impact others. - * - * @param emoji Map from the emoji shortcut (e.g., :+1:) to the image's local path. */ export const buildCustomMarkDownNodeFilterPipe = memoizeOne( - ( - emoji: Map, - repository?: GitHubRepository | undefined, - markdownContext?: MarkdownContext - ): ReadonlyArray => { + (options: ICustomMarkdownFilterOptions): ReadonlyArray => { + const { emoji, repository, markdownContext } = options const filterPipe: Array = [] if (repository !== undefined) { From fca764d11a90ee97411f6934546e8b23e697aac7 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Mon, 23 May 2022 09:47:04 -0400 Subject: [PATCH 07/84] Add feature flag for multi commit diff work --- app/src/lib/feature-flag.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/src/lib/feature-flag.ts b/app/src/lib/feature-flag.ts index 11d5d9ca55..67d44dacb1 100644 --- a/app/src/lib/feature-flag.ts +++ b/app/src/lib/feature-flag.ts @@ -172,3 +172,8 @@ export function enablePullRequestReviewNotifications(): boolean { export function enableReRunFailedAndSingleCheckJobs(): boolean { return true } + +/** Should we enable displaying multi commit diffs. This also switches diff logic from one commit */ +export function enableMultiCommitDiffs(): boolean { + return enableDevelopmentFeatures() +} From 691ca31a1b6a7f25a2250d013fd40447cb4d1e6f Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Mon, 23 May 2022 19:08:41 +0300 Subject: [PATCH 08/84] Add copy paths when multiple files are selected --- app/src/ui/changes/changes-list.tsx | 42 ++++++++++++++++++++++++++--- app/src/ui/lib/context-menu.ts | 6 +++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/app/src/ui/changes/changes-list.tsx b/app/src/ui/changes/changes-list.tsx index a7e0869605..3873d22ee5 100644 --- a/app/src/ui/changes/changes-list.tsx +++ b/app/src/ui/changes/changes-list.tsx @@ -27,6 +27,8 @@ import { RevealInFileManagerLabel, OpenWithDefaultProgramLabel, CopyRelativeFilePathLabel, + CopySelectedPathsLabel, + CopySelectedRelativePathsLabel, } from '../lib/context-menu' import { CommitMessage } from './commit-message' import { ChangedFile } from './changed-file' @@ -426,6 +428,32 @@ export class ChangesList extends React.Component< } } + private getCopySelectedPathsMenuItem = ( + files: WorkingDirectoryFileChange[] + ): IMenuItem => { + return { + label: CopySelectedPathsLabel, + action: () => { + const fullPaths = files.map(file => + Path.join(this.props.repository.path, file.path) + ) + clipboard.writeText(fullPaths.join(' ')) + }, + } + } + + private getCopySelectedRelativePathsMenuItem = ( + files: WorkingDirectoryFileChange[] + ): IMenuItem => { + return { + label: CopySelectedRelativePathsLabel, + action: () => { + const paths = files.map(file => Path.normalize(file.path)) + clipboard.writeText(paths.join(' ')) + }, + } + } + private getRevealInFileManagerMenuItem = ( file: WorkingDirectoryFileChange ): IMenuItem => { @@ -556,15 +584,21 @@ export class ChangesList extends React.Component< this.props.onIncludeChanged(file.path, false) ) }, - } + }, + { type: 'separator' }, + this.getCopySelectedPathsMenuItem(selectedFiles), + this.getCopySelectedRelativePathsMenuItem(selectedFiles) + ) + } else { + items.push( + { type: 'separator' }, + this.getCopyPathMenuItem(file), + this.getCopyRelativePathMenuItem(file) ) } const enabled = status.kind !== AppFileStatusKind.Deleted items.push( - { type: 'separator' }, - this.getCopyPathMenuItem(file), - this.getCopyRelativePathMenuItem(file), { type: 'separator' }, this.getRevealInFileManagerMenuItem(file), this.getOpenInExternalEditorMenuItem(file, enabled), diff --git a/app/src/ui/lib/context-menu.ts b/app/src/ui/lib/context-menu.ts index c92d86aee3..6f96ecb139 100644 --- a/app/src/ui/lib/context-menu.ts +++ b/app/src/ui/lib/context-menu.ts @@ -7,6 +7,12 @@ export const CopyRelativeFilePathLabel = __DARWIN__ ? 'Copy Relative File Path' : 'Copy relative file path' +export const CopySelectedPathsLabel = __DARWIN__ ? 'Copy Paths' : 'Copy paths' + +export const CopySelectedRelativePathsLabel = __DARWIN__ + ? 'Copy Relative Paths' + : 'Copy relative paths' + export const DefaultEditorLabel = __DARWIN__ ? 'Open in External Editor' : 'Open in external editor' From d09178540d88db0d91ea7744b4fae979338d5177 Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Mon, 23 May 2022 20:00:00 +0300 Subject: [PATCH 09/84] Add support for Aptana Studio 3 on Windows --- app/src/lib/editors/win32.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/src/lib/editors/win32.ts b/app/src/lib/editors/win32.ts index b8b2698548..d24776e5ee 100644 --- a/app/src/lib/editors/win32.ts +++ b/app/src/lib/editors/win32.ts @@ -299,6 +299,15 @@ const editors: WindowsExternalEditor[] = [ displayNamePrefix: 'SlickEdit', publisher: 'SlickEdit Inc.', }, + { + name: 'Aptana Studio 3', + registryKeys: [ + Wow64LocalMachineUninstallKey('{2D6C1116-78C6-469C-9923-3E549218773F}'), + ], + executableShimPaths: [['AptanaStudio3.exe']], + displayNamePrefix: 'Aptana Studio', + publisher: 'Appcelerator', + }, { name: 'JetBrains Webstorm', registryKeys: registryKeysForJetBrainsIDE('WebStorm'), From 17bcb2417e04eaf5486847c7c907795507403b17 Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Mon, 23 May 2022 20:33:38 +0300 Subject: [PATCH 10/84] Add Aptana Studio support on MacOS --- app/src/lib/editors/darwin.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/src/lib/editors/darwin.ts b/app/src/lib/editors/darwin.ts index 03c47d3790..15dc96f2d1 100644 --- a/app/src/lib/editors/darwin.ts +++ b/app/src/lib/editors/darwin.ts @@ -23,6 +23,10 @@ const editors: IDarwinExternalEditor[] = [ name: 'Atom', bundleIdentifiers: ['com.github.atom'], }, + { + name: 'Aptana Studio', + bundleIdentifiers: ['aptana.studio'], + }, { name: 'MacVim', bundleIdentifiers: ['org.vim.MacVim'], From 435ce9c0aff61ed9f1bda7588b17179fee8c0507 Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Mon, 23 May 2022 20:41:34 +0300 Subject: [PATCH 11/84] Add Aptana Studio to supported editors list --- docs/technical/editor-integration.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/technical/editor-integration.md b/docs/technical/editor-integration.md index f8ed551741..b6b3084ba5 100644 --- a/docs/technical/editor-integration.md +++ b/docs/technical/editor-integration.md @@ -43,6 +43,7 @@ These editors are currently supported: - [Brackets](http://brackets.io/) - [Notepad++](https://notepad-plus-plus.org/) - [RStudio](https://rstudio.com/) + - [Aptana Studio](http://www.aptana.com/) These are defined in a list at the top of the file: @@ -233,6 +234,7 @@ These editors are currently supported: - [Android Studio](https://developer.android.com/studio) - [JetBrains Rider](https://www.jetbrains.com/rider/) - [Nova](https://nova.app/) + - [Aptana Studio](http://www.aptana.com/) These are defined in a list at the top of the file: From c7973e349415638cc4f860038ae2cb39d6b6e483 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Mon, 23 May 2022 09:50:15 -0400 Subject: [PATCH 12/84] Add method get diff between two commits --- app/src/lib/git/diff.ts | 60 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 7 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index 729954e199..b7741a9d69 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -133,6 +133,52 @@ export async function getCommitDiff( return buildDiff(output, repository, file, commitish) } +/** + * Render the difference between two commits for a file + * + */ +export async function getCommitsDiff( + repository: Repository, + commits: ReadonlyArray, + file: FileChange, + hideWhitespaceInDiff: boolean = false +): Promise { + if (commits.length === 0) { + throw new Error('No commits to diff...') + } + + const commitish = + commits.length === 1 + ? `${commits.at(0)}^..${commits.at(0)}` + : `${commits.at(-1)}..${commits.at(0)}` + const oldestCommit = `${commits.at(-1)}` + const args = [ + 'diff', + commitish, + ...(hideWhitespaceInDiff ? ['-w'] : []), + '--patch-with-raw', + '-z', + '--no-color', + '--', + file.path, + ] + + if ( + file.status.kind === AppFileStatusKind.Renamed || + file.status.kind === AppFileStatusKind.Copied + ) { + args.push(file.status.oldPath) + } + + const { output } = await spawnAndComplete( + args, + repository.path, + 'getCommitsDiff' + ) + + return buildDiff(output, repository, file, oldestCommit) +} + /** * Render the diff for a file within the repository working directory. The file will be * compared against HEAD if it's tracked, if not it'll be compared to an empty file meaning @@ -198,7 +244,7 @@ export async function getWorkingDirectoryDiff( async function getImageDiff( repository: Repository, file: FileChange, - commitish: string + oldestCommitish: string ): Promise { let current: Image | undefined = undefined let previous: Image | undefined = undefined @@ -232,7 +278,7 @@ async function getImageDiff( } else { // File status can't be conflicted for a file in a commit if (file.status.kind !== AppFileStatusKind.Deleted) { - current = await getBlobImage(repository, file.path, commitish) + current = await getBlobImage(repository, file.path, oldestCommitish) } // File status can't be conflicted for a file in a commit @@ -247,7 +293,7 @@ async function getImageDiff( previous = await getBlobImage( repository, getOldPathOrDefault(file), - `${commitish}^` + `${oldestCommitish}^` ) } } @@ -263,7 +309,7 @@ export async function convertDiff( repository: Repository, file: FileChange, diff: IRawDiff, - commitish: string, + oldestCommitish: string, lineEndingsChange?: LineEndingsChange ): Promise { const extension = Path.extname(file.path).toLowerCase() @@ -275,7 +321,7 @@ export async function convertDiff( kind: DiffType.Binary, } } else { - return getImageDiff(repository, file, commitish) + return getImageDiff(repository, file, oldestCommitish) } } @@ -370,7 +416,7 @@ function buildDiff( buffer: Buffer, repository: Repository, file: FileChange, - commitish: string, + oldestCommitish: string, lineEndingsChange?: LineEndingsChange ): Promise { if (!isValidBuffer(buffer)) { @@ -396,7 +442,7 @@ function buildDiff( return Promise.resolve(largeTextDiff) } - return convertDiff(repository, file, diff, commitish, lineEndingsChange) + return convertDiff(repository, file, diff, oldestCommitish, lineEndingsChange) } /** From 115d338e37b83e232b015b375d937c7849461990 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 24 May 2022 12:19:46 -0400 Subject: [PATCH 13/84] Use git (exec) instead of spawn --- app/src/lib/git/diff.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index b7741a9d69..5e0083ecbf 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -27,6 +27,7 @@ import { getOldPathOrDefault } from '../get-old-path' import { getCaptures } from '../helpers/regex' import { readFile } from 'fs/promises' import { forceUnwrap } from '../fatal-error' +import { git } from '.' /** * V8 has a limit on the size of string it can create (~256MB), and unless we want to @@ -170,13 +171,16 @@ export async function getCommitsDiff( args.push(file.status.oldPath) } - const { output } = await spawnAndComplete( - args, - repository.path, - 'getCommitsDiff' - ) + const result = await git(args, repository.path, 'getCommitsDiff', { + maxBuffer: Infinity, + }) - return buildDiff(output, repository, file, oldestCommit) + return buildDiff( + Buffer.from(result.combinedOutput), + repository, + file, + oldestCommit + ) } /** From cab78328fce7c23f02818ffa7ffbf182547bdf9a Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 24 May 2022 12:23:10 -0400 Subject: [PATCH 14/84] Use new commit diff method (feature flagged) --- app/src/lib/git/diff.ts | 2 +- app/src/lib/stores/app-store.ts | 25 ++++++++++++++++++------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index 5e0083ecbf..ca3cf0af0d 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -140,8 +140,8 @@ export async function getCommitDiff( */ export async function getCommitsDiff( repository: Repository, - commits: ReadonlyArray, file: FileChange, + commits: ReadonlyArray, hideWhitespaceInDiff: boolean = false ): Promise { if (commits.length === 0) { diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index ffcb533a5d..9364bd22d8 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -160,6 +160,7 @@ import { appendIgnoreFile, getRepositoryType, RepositoryType, + getCommitsDiff, } from '../git' import { installGlobalLFSFilters, @@ -215,7 +216,10 @@ import { } from './updates/changes-state' import { ManualConflictResolution } from '../../models/manual-conflict-resolution' import { BranchPruner } from './helpers/branch-pruner' -import { enableHideWhitespaceInDiffOption } from '../feature-flag' +import { + enableHideWhitespaceInDiffOption, + enableMultiCommitDiffs, +} from '../feature-flag' import { Banner, BannerType } from '../../models/banner' import { ComputedAction } from '../../models/computed-action' import { @@ -1454,12 +1458,19 @@ export class AppStore extends TypedBaseStore { return } - const diff = await getCommitDiff( - repository, - file, - shas[0], - this.hideWhitespaceInHistoryDiff - ) + const diff = enableMultiCommitDiffs() + ? await getCommitsDiff( + repository, + file, + shas, + this.hideWhitespaceInHistoryDiff + ) + : await getCommitDiff( + repository, + file, + shas[0], + this.hideWhitespaceInHistoryDiff + ) const stateAfterLoad = this.repositoryStateCache.get(repository) const { shas: shasAfter } = stateAfterLoad.commitSelection From eccdd9c364d98cb3bbd7ec1834ead970177a0f8b Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 24 May 2022 13:27:42 -0400 Subject: [PATCH 15/84] Retry diff will null tree sha for bad revisions Update diff.ts --- app/src/lib/git/diff-index.ts | 2 +- app/src/lib/git/diff.ts | 40 +++++++++++++++++++++++++-------- app/src/lib/stores/app-store.ts | 4 ++-- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/app/src/lib/git/diff-index.ts b/app/src/lib/git/diff-index.ts index 599b425c7d..e8b6fd5351 100644 --- a/app/src/lib/git/diff-index.ts +++ b/app/src/lib/git/diff-index.ts @@ -68,7 +68,7 @@ function getNoRenameIndexStatus(status: string): NoRenameIndexStatus { } /** The SHA for the null tree. */ -const NullTreeSHA = '4b825dc642cb6eb9a060e54bf8d69288fbee4904' +export const NullTreeSHA = '4b825dc642cb6eb9a060e54bf8d69288fbee4904' /** * Get a list of files which have recorded changes in the index as compared to diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index ca3cf0af0d..f0acb96ec5 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -28,6 +28,8 @@ import { getCaptures } from '../helpers/regex' import { readFile } from 'fs/promises' import { forceUnwrap } from '../fatal-error' import { git } from '.' +import { NullTreeSHA } from './diff-index' +import { GitError } from 'dugite' /** * V8 has a limit on the size of string it can create (~256MB), and unless we want to @@ -138,24 +140,22 @@ export async function getCommitDiff( * Render the difference between two commits for a file * */ -export async function getCommitsDiff( +export async function getCommitRangeDiff( repository: Repository, file: FileChange, commits: ReadonlyArray, - hideWhitespaceInDiff: boolean = false + hideWhitespaceInDiff: boolean = false, + useNullTreeSHA: boolean = false ): Promise { if (commits.length === 0) { throw new Error('No commits to diff...') } - const commitish = - commits.length === 1 - ? `${commits.at(0)}^..${commits.at(0)}` - : `${commits.at(-1)}..${commits.at(0)}` - const oldestCommit = `${commits.at(-1)}` + const oldestCommitRef = useNullTreeSHA ? NullTreeSHA : `${commits.at(-1)}^` const args = [ 'diff', - commitish, + oldestCommitRef, + commits[0], ...(hideWhitespaceInDiff ? ['-w'] : []), '--patch-with-raw', '-z', @@ -173,13 +173,35 @@ export async function getCommitsDiff( const result = await git(args, repository.path, 'getCommitsDiff', { maxBuffer: Infinity, + expectedErrors: new Set([GitError.BadRevision]), }) + // This should only happen if the oldest commit does not have a parent (ex: + // initial commit of a branch) and therefore `SHA^` is not a valid reference. + // In which case, we will retry with the null tree sha. + if (result.gitError === GitError.BadRevision && useNullTreeSHA === false) { + const useNullTreeSHA = true + return getCommitRangeDiff( + repository, + file, + commits, + hideWhitespaceInDiff, + useNullTreeSHA + ) + } + + if (result.gitError !== null) { + // This shouldn't happen... + throw new Error( + `getCommitRangeDiff: Error in diffing the commit range of: ${oldestCommitRef} to ${commits[0]}` + ) + } + return buildDiff( Buffer.from(result.combinedOutput), repository, file, - oldestCommit + oldestCommitRef ) } diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 9364bd22d8..e818660b4d 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -160,7 +160,7 @@ import { appendIgnoreFile, getRepositoryType, RepositoryType, - getCommitsDiff, + getCommitRangeDiff, } from '../git' import { installGlobalLFSFilters, @@ -1459,7 +1459,7 @@ export class AppStore extends TypedBaseStore { } const diff = enableMultiCommitDiffs() - ? await getCommitsDiff( + ? await getCommitRangeDiff( repository, file, shas, From 772cbfc80d3f45cfaaed5988c0c7cc30231c3ccd Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 24 May 2022 13:52:54 -0400 Subject: [PATCH 16/84] Use tried and true log for single commits --- app/src/lib/stores/app-store.ts | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index e818660b4d..99ac34f8b8 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -1458,19 +1458,20 @@ export class AppStore extends TypedBaseStore { return } - const diff = enableMultiCommitDiffs() - ? await getCommitRangeDiff( - repository, - file, - shas, - this.hideWhitespaceInHistoryDiff - ) - : await getCommitDiff( - repository, - file, - shas[0], - this.hideWhitespaceInHistoryDiff - ) + const diff = + enableMultiCommitDiffs() && shas.length > 1 + ? await getCommitRangeDiff( + repository, + file, + shas, + this.hideWhitespaceInHistoryDiff + ) + : await getCommitDiff( + repository, + file, + shas[0], + this.hideWhitespaceInHistoryDiff + ) const stateAfterLoad = this.repositoryStateCache.get(repository) const { shas: shasAfter } = stateAfterLoad.commitSelection From 560fc5b3995e74074e3765c557c07405d9c616fe Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 24 May 2022 14:12:42 -0400 Subject: [PATCH 17/84] What is this error.. --- app/src/lib/git/diff.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index f0acb96ec5..d3010d4ea6 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -193,7 +193,7 @@ export async function getCommitRangeDiff( if (result.gitError !== null) { // This shouldn't happen... throw new Error( - `getCommitRangeDiff: Error in diffing the commit range of: ${oldestCommitRef} to ${commits[0]}` + `getCommitRangeDiff: Error in diffing the commit range of: ${oldestCommitRef} to ${commits[0]} - ${result.combinedOutput}` ) } From 968beb99c94c849c884226aa9ce57bb131153d0b Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Wed, 25 May 2022 17:12:51 +0200 Subject: [PATCH 18/84] Refactor SSH secret storage functions --- app/src/lib/ssh/ssh-key-passphrase.ts | 51 ++++------------ app/src/lib/ssh/ssh-secret-storage.ts | 61 +++++++++++++++++++ .../trampoline/trampoline-askpass-handler.ts | 6 +- .../lib/trampoline/trampoline-environment.ts | 10 +-- .../lib/trampoline/trampoline-ui-helper.ts | 10 +-- 5 files changed, 87 insertions(+), 51 deletions(-) create mode 100644 app/src/lib/ssh/ssh-secret-storage.ts diff --git a/app/src/lib/ssh/ssh-key-passphrase.ts b/app/src/lib/ssh/ssh-key-passphrase.ts index 3259b48cb1..7782a1f530 100644 --- a/app/src/lib/ssh/ssh-key-passphrase.ts +++ b/app/src/lib/ssh/ssh-key-passphrase.ts @@ -1,8 +1,13 @@ import { getFileHash } from '../file-system' import { TokenStore } from '../stores' +import { + getSSHSecretStoreKey, + keepSSHSecretToStore, +} from './ssh-secret-storage' -const appName = __DEV__ ? 'GitHub Desktop Dev' : 'GitHub Desktop' -const SSHKeyPassphraseTokenStoreKey = `${appName} - SSH key passphrases` +const SSHKeyPassphraseTokenStoreKey = getSSHSecretStoreKey( + 'SSH key passphrases' +) async function getHashForSSHKey(keyPath: string) { return getFileHash(keyPath, 'sha256') @@ -19,22 +24,6 @@ export async function getSSHKeyPassphrase(keyPath: string) { } } -type SSHKeyPassphraseEntry = { - /** Hash of the SSH key file. */ - keyHash: string - - /** Passphrase for the SSH key. */ - passphrase: string -} - -/** - * This map contains the SSH key passphrases that are pending to be stored. - * What this means is that a git operation is currently in progress, and the - * user wanted to store the passphrase for the SSH key, however we don't want - * to store it until we know the git operation finished successfully. - */ -const SSHKeyPassphrasesToStore = new Map() - /** * Keeps the SSH key passphrase in memory to be stored later if the ongoing git * operation succeeds. @@ -52,27 +41,13 @@ export async function keepSSHKeyPassphraseToStore( ) { try { const keyHash = await getHashForSSHKey(keyPath) - SSHKeyPassphrasesToStore.set(operationGUID, { keyHash, passphrase }) + keepSSHSecretToStore( + operationGUID, + SSHKeyPassphraseTokenStoreKey, + keyHash, + passphrase + ) } catch (e) { log.error('Could not store passphrase for SSH key:', e) } } - -/** Removes the SSH key passphrase from memory. */ -export function removePendingSSHKeyPassphraseToStore(operationGUID: string) { - SSHKeyPassphrasesToStore.delete(operationGUID) -} - -/** Stores a pending SSH key passphrase if the operation succeeded. */ -export async function storePendingSSHKeyPassphrase(operationGUID: string) { - const entry = SSHKeyPassphrasesToStore.get(operationGUID) - if (entry === undefined) { - return - } - - await TokenStore.setItem( - SSHKeyPassphraseTokenStoreKey, - entry.keyHash, - entry.passphrase - ) -} diff --git a/app/src/lib/ssh/ssh-secret-storage.ts b/app/src/lib/ssh/ssh-secret-storage.ts new file mode 100644 index 0000000000..7f651591f5 --- /dev/null +++ b/app/src/lib/ssh/ssh-secret-storage.ts @@ -0,0 +1,61 @@ +import { TokenStore } from '../stores' + +const appName = __DEV__ ? 'GitHub Desktop Dev' : 'GitHub Desktop' + +export function getSSHSecretStoreKey(name: string) { + return `${appName} - ${name}` +} + +type SSHSecretEntry = { + /** Store where this entry will be stored. */ + store: string + + /** Key used to identify the secret in the store (e.g. username or hash). */ + key: string + + /** Actual secret to be stored (password). */ + secret: string +} + +/** + * This map contains the SSH secrets that are pending to be stored. What this + * means is that a git operation is currently in progress, and the user wanted + * to store the passphrase for the SSH key, however we don't want to store it + * until we know the git operation finished successfully. + */ +const SSHSecretsToStore = new Map() + +/** + * Keeps the SSH secret in memory to be stored later if the ongoing git operation + * succeeds. + * + * @param operationGUID A unique identifier for the ongoing git operation. In + * practice, it will always be the trampoline secret for the + * ongoing git operation. + * @param key Key that identifies the SSH secret (e.g. username or key + * hash). + * @param secret Actual SSH secret to store. + */ +export async function keepSSHSecretToStore( + operationGUID: string, + store: string, + key: string, + secret: string +) { + SSHSecretsToStore.set(operationGUID, { store, key, secret }) +} + +/** Removes the SSH key passphrase from memory. */ +export function removePendingSSHSecretToStore(operationGUID: string) { + SSHSecretsToStore.delete(operationGUID) +} + +/** Stores a pending SSH key passphrase if the operation succeeded. */ +export async function storePendingSSHSecret(operationGUID: string) { + const entry = SSHSecretsToStore.get(operationGUID) + if (entry === undefined) { + return + } + + await TokenStore.setItem(entry.store, entry.key, entry.secret) +} diff --git a/app/src/lib/trampoline/trampoline-askpass-handler.ts b/app/src/lib/trampoline/trampoline-askpass-handler.ts index 01ee12eccd..307d809fc1 100644 --- a/app/src/lib/trampoline/trampoline-askpass-handler.ts +++ b/app/src/lib/trampoline/trampoline-askpass-handler.ts @@ -2,12 +2,12 @@ import { getKeyForEndpoint } from '../auth' import { getSSHKeyPassphrase, keepSSHKeyPassphraseToStore, - removePendingSSHKeyPassphraseToStore, } from '../ssh/ssh-key-passphrase' import { TokenStore } from '../stores' import { TrampolineCommandHandler } from './trampoline-command' import { trampolineUIHelper } from './trampoline-ui-helper' import { parseAddSSHHostPrompt } from '../ssh/ssh' +import { removePendingSSHSecretToStore } from '../ssh/ssh-secret-storage' async function handleSSHHostAuthenticity( prompt: string @@ -65,7 +65,7 @@ async function handleSSHKeyPassphrase( return storedPassphrase } - const { passphrase, storePassphrase } = + const { secret: passphrase, storeSecret: storePassphrase } = await trampolineUIHelper.promptSSHKeyPassphrase(keyPath) // If the user wanted us to remember the passphrase, we'll keep it around to @@ -78,7 +78,7 @@ async function handleSSHKeyPassphrase( if (passphrase !== undefined && storePassphrase) { keepSSHKeyPassphraseToStore(operationGUID, keyPath, passphrase) } else { - removePendingSSHKeyPassphraseToStore(operationGUID) + removePendingSSHSecretToStore(operationGUID) } return passphrase ?? '' diff --git a/app/src/lib/trampoline/trampoline-environment.ts b/app/src/lib/trampoline/trampoline-environment.ts index 222e18252b..e111a67196 100644 --- a/app/src/lib/trampoline/trampoline-environment.ts +++ b/app/src/lib/trampoline/trampoline-environment.ts @@ -5,9 +5,9 @@ import { getDesktopTrampolineFilename } from 'desktop-trampoline' import { TrampolineCommandIdentifier } from '../trampoline/trampoline-command' import { getSSHEnvironment } from '../ssh/ssh' import { - removePendingSSHKeyPassphraseToStore, - storePendingSSHKeyPassphrase, -} from '../ssh/ssh-key-passphrase' + removePendingSSHSecretToStore, + storePendingSSHSecret, +} from '../ssh/ssh-secret-storage' /** * Allows invoking a function with a set of environment variables to use when @@ -46,11 +46,11 @@ export async function withTrampolineEnv( ...sshEnv, }) - await storePendingSSHKeyPassphrase(token) + await storePendingSSHSecret(token) return result } finally { - removePendingSSHKeyPassphraseToStore(token) + removePendingSSHSecretToStore(token) } }) } diff --git a/app/src/lib/trampoline/trampoline-ui-helper.ts b/app/src/lib/trampoline/trampoline-ui-helper.ts index 52ea64098c..39e71d583f 100644 --- a/app/src/lib/trampoline/trampoline-ui-helper.ts +++ b/app/src/lib/trampoline/trampoline-ui-helper.ts @@ -1,9 +1,9 @@ import { PopupType } from '../../models/popup' import { Dispatcher } from '../../ui/dispatcher' -type PromptSSHKeyPassphraseResponse = { - readonly passphrase: string | undefined - readonly storePassphrase: boolean +type PromptSSHSecretResponse = { + readonly secret: string | undefined + readonly storeSecret: boolean } class TrampolineUIHelper { @@ -34,13 +34,13 @@ class TrampolineUIHelper { public promptSSHKeyPassphrase( keyPath: string - ): Promise { + ): Promise { return new Promise(resolve => { this.dispatcher.showPopup({ type: PopupType.SSHKeyPassphrase, keyPath, onSubmit: (passphrase, storePassphrase) => - resolve({ passphrase, storePassphrase }), + resolve({ secret: passphrase, storeSecret: storePassphrase }), }) }) } From 9a89c27e21654ca64ee3c07e6de9e4e383b912bb Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Wed, 25 May 2022 17:21:03 +0200 Subject: [PATCH 19/84] Add support for SSH password prompts --- app/src/lib/ssh/ssh-user-password.ts | 37 +++++++ .../trampoline/trampoline-askpass-handler.ts | 35 +++++++ .../lib/trampoline/trampoline-ui-helper.ts | 13 +++ app/src/models/popup.ts | 6 ++ app/src/ui/app.tsx | 11 +++ app/src/ui/ssh/ssh-user-password.tsx | 99 +++++++++++++++++++ 6 files changed, 201 insertions(+) create mode 100644 app/src/lib/ssh/ssh-user-password.ts create mode 100644 app/src/ui/ssh/ssh-user-password.tsx diff --git a/app/src/lib/ssh/ssh-user-password.ts b/app/src/lib/ssh/ssh-user-password.ts new file mode 100644 index 0000000000..823fc6f646 --- /dev/null +++ b/app/src/lib/ssh/ssh-user-password.ts @@ -0,0 +1,37 @@ +import { TokenStore } from '../stores' +import { getSSHSecretStoreKey, keepSSHSecretToStore } from './ssh-token-storage' + +const SSHUserPasswordTokenStoreKey = getSSHSecretStoreKey('SSH user password') + +/** Retrieves the password for the given SSH username. */ +export async function getSSHUserPassword(username: string) { + try { + return TokenStore.getItem(SSHUserPasswordTokenStoreKey, username) + } catch (e) { + log.error('Could not retrieve passphrase for SSH key:', e) + return null + } +} + +/** + * Keeps the SSH user password in memory to be stored later if the ongoing git + * operation succeeds. + * + * @param operationGUID A unique identifier for the ongoing git operation. In + * practice, it will always be the trampoline token for the + * ongoing git operation. + * @param username SSH user name. Usually in the form of `user@hostname`. + * @param password Password for the given user. + */ +export async function keepSSHUserPasswordToStore( + operationGUID: string, + username: string, + password: string +) { + keepSSHSecretToStore( + operationGUID, + SSHUserPasswordTokenStoreKey, + username, + password + ) +} diff --git a/app/src/lib/trampoline/trampoline-askpass-handler.ts b/app/src/lib/trampoline/trampoline-askpass-handler.ts index 307d809fc1..68d6cc62a9 100644 --- a/app/src/lib/trampoline/trampoline-askpass-handler.ts +++ b/app/src/lib/trampoline/trampoline-askpass-handler.ts @@ -7,6 +7,10 @@ import { TokenStore } from '../stores' import { TrampolineCommandHandler } from './trampoline-command' import { trampolineUIHelper } from './trampoline-ui-helper' import { parseAddSSHHostPrompt } from '../ssh/ssh' +import { + getSSHUserPassword, + keepSSHUserPasswordToStore, +} from '../ssh/ssh-user-password' import { removePendingSSHSecretToStore } from '../ssh/ssh-secret-storage' async function handleSSHHostAuthenticity( @@ -84,6 +88,33 @@ async function handleSSHKeyPassphrase( return passphrase ?? '' } +async function handleSSHUserPassword(operationGUID: string, prompt: string) { + const promptRegex = /^(.+@.+)'s password: $/ + + const matches = promptRegex.exec(prompt) + if (matches === null || matches.length < 2) { + return undefined + } + + const username = matches[1] + + const storedPassword = await getSSHUserPassword(username) + if (storedPassword !== null) { + return storedPassword + } + + const { secret: password, storeSecret: storePassword } = + await trampolineUIHelper.promptSSHUserPassword(username) + + if (password !== undefined && storePassword) { + keepSSHUserPasswordToStore(operationGUID, username, password) + } else { + removePendingSSHSecretToStore(operationGUID) + } + + return password ?? '' +} + export const askpassTrampolineHandler: TrampolineCommandHandler = async command => { if (command.parameters.length !== 1) { @@ -100,6 +131,10 @@ export const askpassTrampolineHandler: TrampolineCommandHandler = return handleSSHKeyPassphrase(command.trampolineToken, firstParameter) } + if (firstParameter.endsWith("'s password: ")) { + return handleSSHUserPassword(command.trampolineToken, firstParameter) + } + const username = command.environmentVariables.get('DESKTOP_USERNAME') if (username === undefined || username.length === 0) { return undefined diff --git a/app/src/lib/trampoline/trampoline-ui-helper.ts b/app/src/lib/trampoline/trampoline-ui-helper.ts index 39e71d583f..7e6fe66d5e 100644 --- a/app/src/lib/trampoline/trampoline-ui-helper.ts +++ b/app/src/lib/trampoline/trampoline-ui-helper.ts @@ -44,6 +44,19 @@ class TrampolineUIHelper { }) }) } + + public promptSSHUserPassword( + username: string + ): Promise { + return new Promise(resolve => { + this.dispatcher.showPopup({ + type: PopupType.SSHUserPassword, + username, + onSubmit: (password, storePassword) => + resolve({ secret: password, storeSecret: storePassword }), + }) + }) + } } export const trampolineUIHelper = new TrampolineUIHelper() diff --git a/app/src/models/popup.ts b/app/src/models/popup.ts index a312292002..fdf1058b3a 100644 --- a/app/src/models/popup.ts +++ b/app/src/models/popup.ts @@ -78,6 +78,7 @@ export enum PopupType { InvalidatedToken, AddSSHHost, SSHKeyPassphrase, + SSHUserPassword, PullRequestChecksFailed, CICheckRunRerun, WarnForcePush, @@ -317,6 +318,11 @@ export type Popup = storePassphrase: boolean ) => void } + | { + type: PopupType.SSHUserPassword + username: string + onSubmit: (password: string | undefined, storePassword: boolean) => void + } | { type: PopupType.PullRequestChecksFailed repository: RepositoryWithGitHubRepository diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index 4e0d24fcf5..6140020c25 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -154,6 +154,7 @@ import { generateDevReleaseSummary } from '../lib/release-notes' 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' const MinuteInMilliseconds = 1000 * 60 const HourInMilliseconds = MinuteInMilliseconds * 60 @@ -2113,6 +2114,16 @@ export class App extends React.Component { /> ) } + case PopupType.SSHUserPassword: { + return ( + + ) + } case PopupType.PullRequestChecksFailed: { return ( void + readonly onDismissed: () => void +} + +interface ISSHUserPasswordState { + readonly password: string + readonly rememberPassword: boolean +} + +/** + * Dialog prompts the user the password of an SSH user. + */ +export class SSHUserPassword extends React.Component< + ISSHUserPasswordProps, + ISSHUserPasswordState +> { + public constructor(props: ISSHUserPasswordProps) { + super(props) + this.state = { password: '', rememberPassword: false } + } + + public render() { + return ( + + + + + + + + + + + + + + ) + } + + private onRememberPasswordChanged = ( + event: React.FormEvent + ) => { + this.setState({ rememberPassword: event.currentTarget.checked }) + } + + private onValueChanged = (value: string) => { + this.setState({ password: value }) + } + + private submit(password: string | undefined, storePassword: boolean) { + const { onSubmit, onDismissed } = this.props + + onSubmit(password, storePassword) + onDismissed() + } + + private onSubmit = () => { + this.submit(this.state.password, this.state.rememberPassword) + } + + private onCancel = () => { + this.submit(undefined, false) + } +} From 7b6f61d8512d8f19be42c664ed0d5ce85b2f9fe8 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Wed, 25 May 2022 17:56:14 +0200 Subject: [PATCH 20/84] =?UTF-8?q?Add=20changes=20I=20forgot=20to=20save?= =?UTF-8?q?=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/src/lib/ssh/ssh-user-password.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/src/lib/ssh/ssh-user-password.ts b/app/src/lib/ssh/ssh-user-password.ts index 823fc6f646..4bb25c4301 100644 --- a/app/src/lib/ssh/ssh-user-password.ts +++ b/app/src/lib/ssh/ssh-user-password.ts @@ -1,5 +1,8 @@ import { TokenStore } from '../stores' -import { getSSHSecretStoreKey, keepSSHSecretToStore } from './ssh-token-storage' +import { + getSSHSecretStoreKey, + keepSSHSecretToStore, +} from './ssh-secret-storage' const SSHUserPasswordTokenStoreKey = getSSHSecretStoreKey('SSH user password') From a7b48a22d8b2d951600db662a8f3f5cc49be03c7 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 May 2022 12:02:18 -0400 Subject: [PATCH 21/84] Remove unnecessary async identifiers --- app/src/ui/lib/sandboxed-markdown.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/ui/lib/sandboxed-markdown.tsx b/app/src/ui/lib/sandboxed-markdown.tsx index 9b02638313..561f46a09c 100644 --- a/app/src/ui/lib/sandboxed-markdown.tsx +++ b/app/src/ui/lib/sandboxed-markdown.tsx @@ -118,7 +118,7 @@ export class SandboxedMarkdown extends React.PureComponent< this.frameContainingDivRef = frameContainingDivRef } - private initializeMarkdownEmitter = async () => { + private initializeMarkdownEmitter = () => { if (this.markdownEmitter !== undefined) { this.markdownEmitter.dispose() } @@ -138,7 +138,7 @@ export class SandboxedMarkdown extends React.PureComponent< } public async componentDidMount() { - await this.initializeMarkdownEmitter() + this.initializeMarkdownEmitter() if (this.frameRef !== null) { this.setupFrameLoadListeners(this.frameRef) From 8711405f189cad4232d0bb1bb2af97f28e754824 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 May 2022 12:16:28 -0400 Subject: [PATCH 22/84] Put some disposed emitter breaks in --- app/src/lib/markdown-filters/node-filter.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/src/lib/markdown-filters/node-filter.ts b/app/src/lib/markdown-filters/node-filter.ts index cd8264ac0c..170d4e096e 100644 --- a/app/src/lib/markdown-filters/node-filter.ts +++ b/app/src/lib/markdown-filters/node-filter.ts @@ -108,7 +108,7 @@ export async function applyNodeFilters( nodeFilters: ReadonlyArray, markdownEmitter: MarkdownEmitter ): Promise { - if (markdownEmitter.latestMarkdown === null) { + if (markdownEmitter.latestMarkdown === null || markdownEmitter.isDisposed) { return } @@ -119,6 +119,9 @@ export async function applyNodeFilters( for (const nodeFilter of nodeFilters) { await applyNodeFilter(nodeFilter, mdDoc) + if (markdownEmitter.isDisposed) { + break + } markdownEmitter.emit(mdDoc.documentElement.innerHTML) } } From da1d47b11c1156198ff617a02b7e61b49cc97ce4 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 May 2022 14:41:48 -0400 Subject: [PATCH 23/84] That error will already be handled by the `git` call --- app/src/lib/git/diff.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index d3010d4ea6..8cb67ddeda 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -190,13 +190,6 @@ export async function getCommitRangeDiff( ) } - if (result.gitError !== null) { - // This shouldn't happen... - throw new Error( - `getCommitRangeDiff: Error in diffing the commit range of: ${oldestCommitRef} to ${commits[0]} - ${result.combinedOutput}` - ) - } - return buildDiff( Buffer.from(result.combinedOutput), repository, From 43fc7a0e1c6c3dbe90d4a8d348d450997d71b117 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 May 2022 12:56:13 -0400 Subject: [PATCH 24/84] Use diff method for > 1 commit Update log.ts --- app/src/lib/git/diff.ts | 8 ++++++++ app/src/lib/stores/app-store.ts | 10 ++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index d3010d4ea6..8e15028d04 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -205,6 +205,14 @@ export async function getCommitRangeDiff( ) } +export function getCommitRangeChangedFiles( + repository: Repository, + shas: ReadonlyArray, + useNullTreeSHA: boolean = false +) { + return { files: [], linesAdded: 0, linesDeleted: 0 } +} + /** * Render the diff for a file within the repository working directory. The file will be * compared against HEAD if it's tracked, if not it'll be compared to an empty file meaning diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 99ac34f8b8..98745877d3 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -161,6 +161,7 @@ import { getRepositoryType, RepositoryType, getCommitRangeDiff, + getCommitRangeChangedFiles, } from '../git' import { installGlobalLFSFilters, @@ -1377,14 +1378,15 @@ export class AppStore extends TypedBaseStore { const state = this.repositoryStateCache.get(repository) const { commitSelection } = state const currentSHAs = commitSelection.shas - if (currentSHAs.length !== 1) { - // if none or multiple, we don't display a diff + if (currentSHAs.length === 0) { return } const gitStore = this.gitStoreCache.get(repository) const changesetData = await gitStore.performFailableOperation(() => - getChangedFiles(repository, currentSHAs[0]) + currentSHAs.length > 1 + ? getCommitRangeChangedFiles(repository, currentSHAs) + : getChangedFiles(repository, currentSHAs[0]) ) if (!changesetData) { return @@ -1395,7 +1397,7 @@ export class AppStore extends TypedBaseStore { // SHA/path. if ( commitSelection.shas.length !== currentSHAs.length || - commitSelection.shas[0] !== currentSHAs[0] + !commitSelection.shas.every((sha, i) => sha === currentSHAs[i]) ) { return } From 4618294e264c7af0b5f680526795c846c0867642 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 May 2022 14:15:40 -0400 Subject: [PATCH 25/84] Get changed files for commit range using diff --- app/src/lib/git/diff.ts | 115 ++++++++++++++++++++++++++++++++++++++-- app/src/lib/git/log.ts | 2 +- 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index 8e15028d04..8f028ce1a6 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -7,6 +7,7 @@ import { WorkingDirectoryFileChange, FileChange, AppFileStatusKind, + CommittedFileChange, } from '../../models/status' import { DiffType, @@ -27,7 +28,7 @@ import { getOldPathOrDefault } from '../get-old-path' import { getCaptures } from '../helpers/regex' import { readFile } from 'fs/promises' import { forceUnwrap } from '../fatal-error' -import { git } from '.' +import { git, mapStatus } from '.' import { NullTreeSHA } from './diff-index' import { GitError } from 'dugite' @@ -205,12 +206,120 @@ export async function getCommitRangeDiff( ) } -export function getCommitRangeChangedFiles( +export async function getCommitRangeChangedFiles( repository: Repository, shas: ReadonlyArray, useNullTreeSHA: boolean = false ) { - return { files: [], linesAdded: 0, linesDeleted: 0 } + if (shas.length === 0) { + throw new Error('No commits to diff...') + } + + const oldestCommitRef = useNullTreeSHA ? NullTreeSHA : `${shas.at(-1)}^` + const baseArgs = [ + 'diff', + oldestCommitRef, + shas[0], + '-C', + '-M', + '-z', + '--raw', + '--numstat', + ] + + const result = await git( + baseArgs, + repository.path, + 'getCommitRangeChangedFiles' + ) + + return parseChangedFilesAndNumStat( + result.combinedOutput, + `${oldestCommitRef}..${shas[0]}` + ) +} + +/** + * Parses output of diff flags -z --raw --numstat. + * + * Given the -z flag the new lines are separated by \0 character (left them as + * new lines below for ease of reading) + * + * For modified, added, deleted, untracked: + * 100644 100644 5716ca5 db3c77d M + * file_one_path + * :100644 100644 0835e4f 28096ea M + * file_two_path + * 1 0 file_one_path + * 1 0 file_two_path + * + * For copied or renamed: + * 100644 100644 5716ca5 db3c77d M + * file_one_original_path + * file_one_new_path + * :100644 100644 0835e4f 28096ea M + * file_two_original_path + * file_two_new_path + * 1 0 + * file_one_original_path + * file_one_new_path + * 1 0 + * file_two_original_path + * file_two_new_path + */ +function parseChangedFilesAndNumStat(stdout: string, committish: string) { + const lines = stdout.split('\0') + // Remove the trailing empty line + lines.splice(-1, 1) + + const files: CommittedFileChange[] = [] + let totalLinesAdded = 0 + let totalLinesDeleted = 0 + + for (let i = 0; i < lines.length; i++) { + const parts = lines[i].split('\t') + + if (parts.length === 1) { + const statusParts = parts[0].split(' ') + const statusText = statusParts.at(-1) ?? '' + let oldPath: string | undefined = undefined + + if ( + statusText.length > 0 && + (statusText[0] === 'R' || statusText[0] === 'C') + ) { + oldPath = lines[++i] + } + + const status = mapStatus(statusText, oldPath) + const path = lines[++i] + + files.push(new CommittedFileChange(path, status, committish)) + } + + if (parts.length === 3) { + const [added, deleted, file] = parts + + if (added === '-' || deleted === '-') { + continue + } + + totalLinesAdded += parseInt(added, 10) + totalLinesDeleted += parseInt(deleted, 10) + + // If a file is not renamed or copied, the file name is with the + // add/deleted lines other wise the 2 files names are the next two lines + if (file === '' && lines[i + 1].split('\t').length === 1) { + i = i + 2 + } + } + } + + return { + files, + linesAdded: totalLinesAdded, + linesDeleted: totalLinesDeleted, + } } /** diff --git a/app/src/lib/git/log.ts b/app/src/lib/git/log.ts index 0ae17d33cd..0de475dc8f 100644 --- a/app/src/lib/git/log.ts +++ b/app/src/lib/git/log.ts @@ -22,7 +22,7 @@ import { enableLineChangesInCommit } from '../feature-flag' * Map the raw status text from Git to an app-friendly value * shamelessly borrowed from GitHub Desktop (Windows) */ -function mapStatus( +export function mapStatus( rawStatus: string, oldPath?: string ): PlainFileStatus | CopiedOrRenamedFileStatus | UntrackedFileStatus { From 8d1e2216d3de324a7f10310abb333d8e3e536a41 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 May 2022 14:29:16 -0400 Subject: [PATCH 26/84] Better flag placement --- app/src/lib/stores/app-store.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 98745877d3..c26b526475 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -1378,7 +1378,10 @@ export class AppStore extends TypedBaseStore { const state = this.repositoryStateCache.get(repository) const { commitSelection } = state const currentSHAs = commitSelection.shas - if (currentSHAs.length === 0) { + if ( + currentSHAs.length === 0 || + (currentSHAs.length !== 1 && !enableMultiCommitDiffs()) + ) { return } From 21fcd960fc83202feb94c0151fbbb05831b6c543 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 May 2022 14:40:39 -0400 Subject: [PATCH 27/84] Add check for bad revision --- app/src/lib/git/diff.ts | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index 8f028ce1a6..cae2f56826 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -210,7 +210,11 @@ export async function getCommitRangeChangedFiles( repository: Repository, shas: ReadonlyArray, useNullTreeSHA: boolean = false -) { +): Promise<{ + files: ReadonlyArray + linesAdded: number + linesDeleted: number +}> { if (shas.length === 0) { throw new Error('No commits to diff...') } @@ -225,14 +229,26 @@ export async function getCommitRangeChangedFiles( '-z', '--raw', '--numstat', + '--', ] const result = await git( baseArgs, repository.path, - 'getCommitRangeChangedFiles' + 'getCommitRangeChangedFiles', + { + expectedErrors: new Set([GitError.BadRevision]), + } ) + // This should only happen if the oldest commit does not have a parent (ex: + // initial commit of a branch) and therefore `SHA^` is not a valid reference. + // In which case, we will retry with the null tree sha. + if (result.gitError === GitError.BadRevision && useNullTreeSHA === false) { + const useNullTreeSHA = true + return getCommitRangeChangedFiles(repository, shas, useNullTreeSHA) + } + return parseChangedFilesAndNumStat( result.combinedOutput, `${oldestCommitRef}..${shas[0]}` From 763f791a3a375cfc2d7be00062c64faad43f0096 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 May 2022 14:40:48 -0400 Subject: [PATCH 28/84] Tidying --- app/src/lib/git/diff.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index cae2f56826..9c91e60903 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -289,8 +289,8 @@ function parseChangedFilesAndNumStat(stdout: string, committish: string) { lines.splice(-1, 1) const files: CommittedFileChange[] = [] - let totalLinesAdded = 0 - let totalLinesDeleted = 0 + let linesAdded = 0 + let linesDeleted = 0 for (let i = 0; i < lines.length; i++) { const parts = lines[i].split('\t') @@ -320,8 +320,8 @@ function parseChangedFilesAndNumStat(stdout: string, committish: string) { continue } - totalLinesAdded += parseInt(added, 10) - totalLinesDeleted += parseInt(deleted, 10) + linesAdded += parseInt(added, 10) + linesDeleted += parseInt(deleted, 10) // If a file is not renamed or copied, the file name is with the // add/deleted lines other wise the 2 files names are the next two lines @@ -333,8 +333,8 @@ function parseChangedFilesAndNumStat(stdout: string, committish: string) { return { files, - linesAdded: totalLinesAdded, - linesDeleted: totalLinesDeleted, + linesAdded, + linesDeleted, } } From eba06ef67a0779a08f087dc0e265bedede92604a Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 26 May 2022 12:48:25 +0200 Subject: [PATCH 29/84] Bump @types/event-kit and fix issues - Replaced `IDisposable` with `DisposableLike` - Check `disposed` instead of `isDisposed` (which doesn't exist in runtime) --- app/src/lib/markdown-filters/node-filter.ts | 4 ++-- app/src/lib/stores/ahead-behind-store.ts | 4 ++-- app/src/lib/stores/commit-status-store.ts | 4 ++-- app/src/ui/branches/ci-status.tsx | 4 ++-- app/src/ui/check-runs/ci-check-run-popover.tsx | 4 ++-- app/src/ui/dispatcher/dispatcher.ts | 4 ++-- app/src/ui/history/compare-branch-list-item.tsx | 4 ++-- package.json | 2 +- yarn.lock | 8 ++++---- 9 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/src/lib/markdown-filters/node-filter.ts b/app/src/lib/markdown-filters/node-filter.ts index 170d4e096e..83ad978260 100644 --- a/app/src/lib/markdown-filters/node-filter.ts +++ b/app/src/lib/markdown-filters/node-filter.ts @@ -108,7 +108,7 @@ export async function applyNodeFilters( nodeFilters: ReadonlyArray, markdownEmitter: MarkdownEmitter ): Promise { - if (markdownEmitter.latestMarkdown === null || markdownEmitter.isDisposed) { + if (markdownEmitter.latestMarkdown === null || markdownEmitter.disposed) { return } @@ -119,7 +119,7 @@ export async function applyNodeFilters( for (const nodeFilter of nodeFilters) { await applyNodeFilter(nodeFilter, mdDoc) - if (markdownEmitter.isDisposed) { + if (markdownEmitter.disposed) { break } markdownEmitter.emit(mdDoc.documentElement.innerHTML) diff --git a/app/src/lib/stores/ahead-behind-store.ts b/app/src/lib/stores/ahead-behind-store.ts index c541af9315..12dbc4d219 100644 --- a/app/src/lib/stores/ahead-behind-store.ts +++ b/app/src/lib/stores/ahead-behind-store.ts @@ -1,6 +1,6 @@ import pLimit from 'p-limit' import QuickLRU from 'quick-lru' -import { IDisposable, Disposable } from 'event-kit' +import { DisposableLike, Disposable } from 'event-kit' import { IAheadBehind } from '../../models/branch' import { revSymmetricDifference, getAheadBehind } from '../git' import { Repository } from '../../models/repository' @@ -76,7 +76,7 @@ export class AheadBehindStore { from: string, to: string, callback: AheadBehindCallback - ): IDisposable { + ): DisposableLike { const key = getCacheKey(repository, from, to) const existing = this.cache.get(key) const disposable = new Disposable(() => {}) diff --git a/app/src/lib/stores/commit-status-store.ts b/app/src/lib/stores/commit-status-store.ts index a3f1f913ad..a43ab95bc1 100644 --- a/app/src/lib/stores/commit-status-store.ts +++ b/app/src/lib/stores/commit-status-store.ts @@ -5,7 +5,7 @@ import { Account } from '../../models/account' import { AccountsStore } from './accounts-store' import { GitHubRepository } from '../../models/github-repository' import { API, getAccountForEndpoint, IAPICheckSuite } from '../api' -import { IDisposable, Disposable } from 'event-kit' +import { DisposableLike, Disposable } from 'event-kit' import { ICombinedRefCheck, IRefCheck, @@ -465,7 +465,7 @@ export class CommitStatusStore { ref: string, callback: StatusCallBack, branchName?: string - ): IDisposable { + ): DisposableLike { const key = getCacheKeyForRepository(repository, ref) const subscription = this.getOrCreateSubscription( repository, diff --git a/app/src/ui/branches/ci-status.tsx b/app/src/ui/branches/ci-status.tsx index ffe530f2c5..994062ce33 100644 --- a/app/src/ui/branches/ci-status.tsx +++ b/app/src/ui/branches/ci-status.tsx @@ -3,7 +3,7 @@ import { Octicon, OcticonSymbolType } from '../octicons' import * as OcticonSymbol from '../octicons/octicons.generated' import classNames from 'classnames' import { GitHubRepository } from '../../models/github-repository' -import { IDisposable } from 'event-kit' +import { DisposableLike } from 'event-kit' import { Dispatcher } from '../dispatcher' import { ICombinedRefCheck, @@ -37,7 +37,7 @@ export class CIStatus extends React.PureComponent< ICIStatusProps, ICIStatusState > { - private statusSubscription: IDisposable | null = null + private statusSubscription: DisposableLike | null = null public constructor(props: ICIStatusProps) { super(props) diff --git a/app/src/ui/check-runs/ci-check-run-popover.tsx b/app/src/ui/check-runs/ci-check-run-popover.tsx index 695c7e7837..9a1a47dcdb 100644 --- a/app/src/ui/check-runs/ci-check-run-popover.tsx +++ b/app/src/ui/check-runs/ci-check-run-popover.tsx @@ -1,6 +1,6 @@ import * as React from 'react' import { GitHubRepository } from '../../models/github-repository' -import { IDisposable } from 'event-kit' +import { DisposableLike } from 'event-kit' import { Dispatcher } from '../dispatcher' import { getCheckRunConclusionAdjective, @@ -62,7 +62,7 @@ export class CICheckRunPopover extends React.PureComponent< ICICheckRunPopoverProps, ICICheckRunPopoverState > { - private statusSubscription: IDisposable | null = null + private statusSubscription: DisposableLike | null = null public constructor(props: ICICheckRunPopoverProps) { super(props) diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index 84fbc65eb9..0859b064bd 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -1,4 +1,4 @@ -import { Disposable, IDisposable } from 'event-kit' +import { Disposable, DisposableLike } from 'event-kit' import { IAPIOrganization, @@ -2500,7 +2500,7 @@ export class Dispatcher { ref: string, callback: StatusCallBack, branchName?: string - ): IDisposable { + ): DisposableLike { return this.commitStatusStore.subscribe( repository, ref, diff --git a/app/src/ui/history/compare-branch-list-item.tsx b/app/src/ui/history/compare-branch-list-item.tsx index 30a01d7195..4adb6949f5 100644 --- a/app/src/ui/history/compare-branch-list-item.tsx +++ b/app/src/ui/history/compare-branch-list-item.tsx @@ -7,7 +7,7 @@ import { Branch, IAheadBehind } from '../../models/branch' import { IMatches } from '../../lib/fuzzy-find' import { AheadBehindStore } from '../../lib/stores/ahead-behind-store' import { Repository } from '../../models/repository' -import { IDisposable } from 'event-kit' +import { DisposableLike } from 'event-kit' interface ICompareBranchListItemProps { readonly branch: Branch @@ -50,7 +50,7 @@ export class CompareBranchListItem extends React.Component< return { aheadBehind, comparisonFrom: from, comparisonTo: to } } - private aheadBehindSubscription: IDisposable | null = null + private aheadBehindSubscription: DisposableLike | null = null public constructor(props: ICompareBranchListItemProps) { super(props) diff --git a/package.json b/package.json index 12e0162338..bc585167be 100644 --- a/package.json +++ b/package.json @@ -116,7 +116,7 @@ "@types/electron-winstaller": "^4.0.0", "@types/eslint": "^8.4.1", "@types/estree": "^0.0.49", - "@types/event-kit": "^1.2.28", + "@types/event-kit": "^2.4.1", "@types/express": "^4.11.0", "@types/fs-extra": "^7.0.0", "@types/fuzzaldrin-plus": "^0.0.1", diff --git a/yarn.lock b/yarn.lock index d9e6bee636..b9f0b26949 100644 --- a/yarn.lock +++ b/yarn.lock @@ -960,10 +960,10 @@ resolved "https://registry.yarnpkg.com/@types/estree/-/estree-0.0.50.tgz#1e0caa9364d3fccd2931c3ed96fdbeaa5d4cca83" integrity sha512-C6N5s2ZFtuZRj54k2/zyRhNDjJwwcViAM3Nbm8zjBpbqAdZ00mr0CFxvSKeO8Y/e03WVFLpQMdHYVfUd6SB+Hw== -"@types/event-kit@^1.2.28": - version "1.2.32" - resolved "https://registry.yarnpkg.com/@types/event-kit/-/event-kit-1.2.32.tgz#068cbdc69e8c969afae8c9f6e3a51ea4b1b5522e" - integrity sha512-v+dvA/8Uqp5OfLkd8PRPCZgIWyfz2n14yZdyHvMkZG3Kl4d5K/7son3w18p9bh8zXx3FeT5/DZnu3cM8dWh3sg== +"@types/event-kit@^2.4.1": + version "2.4.1" + resolved "https://registry.yarnpkg.com/@types/event-kit/-/event-kit-2.4.1.tgz#cc00a9b80bae9a387ea60d5c9031b5eb490cfa34" + integrity sha512-ZwGAHGQSj+ZRmqueYyjfIrXRfwLd5A2Z0mfzpP40M9F+BlbUI0v7qsVVFHcWNTE+rq5TLzHeFhEGwFp1zZBSUQ== "@types/events@*": version "1.2.0" From 1978578c2562b25635f6a0a8d570d4c2db837bff Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 26 May 2022 14:00:09 +0200 Subject: [PATCH 30/84] Bump Dexie to version 3.2.2 --- app/package.json | 2 +- app/src/lib/databases/base-database.ts | 4 ++-- app/src/lib/databases/issues-database.ts | 4 ++-- app/src/lib/databases/repositories-database.ts | 8 ++++---- app/yarn.lock | 8 ++++---- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/package.json b/app/package.json index a2419b2d09..0acb4b9297 100644 --- a/app/package.json +++ b/app/package.json @@ -28,7 +28,7 @@ "deep-equal": "^1.0.1", "desktop-notifications": "^0.2.0", "desktop-trampoline": "desktop/desktop-trampoline#v0.9.8", - "dexie": "^2.0.0", + "dexie": "^3.2.2", "dompurify": "^2.3.3", "dugite": "^1.109.0", "electron-window-state": "^5.0.3", diff --git a/app/src/lib/databases/base-database.ts b/app/src/lib/databases/base-database.ts index 1189901eea..c1ccfe2c08 100644 --- a/app/src/lib/databases/base-database.ts +++ b/app/src/lib/databases/base-database.ts @@ -1,4 +1,4 @@ -import Dexie from 'dexie' +import Dexie, { Transaction } from 'dexie' export abstract class BaseDatabase extends Dexie { private schemaVersion: number | undefined @@ -23,7 +23,7 @@ export abstract class BaseDatabase extends Dexie { protected async conditionalVersion( version: number, schema: { [key: string]: string | null }, - upgrade?: (t: Dexie.Transaction) => Promise + upgrade?: (t: Transaction) => Promise ) { if (this.schemaVersion != null && this.schemaVersion < version) { return diff --git a/app/src/lib/databases/issues-database.ts b/app/src/lib/databases/issues-database.ts index 4eafd591d5..1bf8c95225 100644 --- a/app/src/lib/databases/issues-database.ts +++ b/app/src/lib/databases/issues-database.ts @@ -1,4 +1,4 @@ -import Dexie from 'dexie' +import Dexie, { Transaction } from 'dexie' import { BaseDatabase } from './base-database' export interface IIssue { @@ -37,7 +37,7 @@ export class IssuesDatabase extends BaseDatabase { } } -function clearIssues(transaction: Dexie.Transaction) { +function clearIssues(transaction: Transaction) { // Clear deprecated localStorage keys, we compute the since parameter // using the database now. clearDeprecatedKeys() diff --git a/app/src/lib/databases/repositories-database.ts b/app/src/lib/databases/repositories-database.ts index 36b6e7fbf8..76fbebd833 100644 --- a/app/src/lib/databases/repositories-database.ts +++ b/app/src/lib/databases/repositories-database.ts @@ -1,4 +1,4 @@ -import Dexie from 'dexie' +import Dexie, { Transaction } from 'dexie' import { BaseDatabase } from './base-database' import { WorkflowPreferences } from '../../models/workflow-preferences' import { assertNonNullable } from '../fatal-error' @@ -144,7 +144,7 @@ export class RepositoriesDatabase extends BaseDatabase { /** * Remove any duplicate GitHub repositories that have the same owner and name. */ -function removeDuplicateGitHubRepositories(transaction: Dexie.Transaction) { +function removeDuplicateGitHubRepositories(transaction: Transaction) { const table = transaction.table( 'gitHubRepositories' ) @@ -164,7 +164,7 @@ function removeDuplicateGitHubRepositories(transaction: Dexie.Transaction) { }) } -async function ensureNoUndefinedParentID(tx: Dexie.Transaction) { +async function ensureNoUndefinedParentID(tx: Transaction) { return tx .table('gitHubRepositories') .toCollection() @@ -185,7 +185,7 @@ async function ensureNoUndefinedParentID(tx: Dexie.Transaction) { * (https://github.com/desktop/desktop/pull/1242). This scenario ought to be * incredibly unlikely. */ -async function createOwnerKey(tx: Dexie.Transaction) { +async function createOwnerKey(tx: Transaction) { const ownersTable = tx.table('owners') const ghReposTable = tx.table( 'gitHubRepositories' diff --git a/app/yarn.lock b/app/yarn.lock index 1b74232ad6..8e44d894eb 100644 --- a/app/yarn.lock +++ b/app/yarn.lock @@ -398,10 +398,10 @@ devtron@^1.4.0: highlight.js "^9.3.0" humanize-plus "^1.8.1" -dexie@^2.0.0: - version "2.0.4" - resolved "https://registry.yarnpkg.com/dexie/-/dexie-2.0.4.tgz#6027a5e05879424e8f9979d8c14e7420f27e3a11" - integrity sha512-aQ/s1U2wHxwBKRrt2Z/mwFNHMQWhESerFsMYzE+5P5OsIe5o1kgpFMWkzKTtkvkyyEni6mWr/T4HUJuY9xIHLA== +dexie@^3.2.2: + version "3.2.2" + resolved "https://registry.yarnpkg.com/dexie/-/dexie-3.2.2.tgz#fa6f2a3c0d6ed0766f8d97a03720056f88fe0e01" + integrity sha512-q5dC3HPmir2DERlX+toCBbHQXW5MsyrFqPFcovkH9N2S/UW/H3H5AWAB6iEOExeraAu+j+zRDG+zg/D7YhH0qg== dom-classlist@^1.0.1: version "1.0.1" From 584e25ef59b3944a7e2560628f1381ef44d29bac Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 26 May 2022 12:23:18 -0400 Subject: [PATCH 31/84] Had my earliest and latest commits inverted.. --- app/src/lib/git/diff.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index 8cb67ddeda..9b270fb5cc 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -151,11 +151,12 @@ export async function getCommitRangeDiff( throw new Error('No commits to diff...') } - const oldestCommitRef = useNullTreeSHA ? NullTreeSHA : `${commits.at(-1)}^` + const oldestCommitRef = useNullTreeSHA ? NullTreeSHA : `${commits[0]}^` + const latestCommit = commits.at(-1) ?? '' // can't be undefined since commits.length > 0 const args = [ 'diff', oldestCommitRef, - commits[0], + latestCommit, ...(hideWhitespaceInDiff ? ['-w'] : []), '--patch-with-raw', '-z', @@ -194,7 +195,7 @@ export async function getCommitRangeDiff( Buffer.from(result.combinedOutput), repository, file, - oldestCommitRef + latestCommit ) } From 8621d54d09f8f57336e65a2f246852dcfc028821 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 26 May 2022 12:25:47 -0400 Subject: [PATCH 32/84] Had my commit order inverted --- app/src/lib/git/diff.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index c2afab0802..7de22680c5 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -213,11 +213,12 @@ export async function getCommitRangeChangedFiles( throw new Error('No commits to diff...') } - const oldestCommitRef = useNullTreeSHA ? NullTreeSHA : `${shas.at(-1)}^` + const oldestCommitRef = useNullTreeSHA ? NullTreeSHA : `${shas[0]}^` + const latestCommitRef = shas.at(-1) ?? '' // can't be undefined since shas.length > 0 const baseArgs = [ 'diff', oldestCommitRef, - shas[0], + latestCommitRef, '-C', '-M', '-z', @@ -245,7 +246,7 @@ export async function getCommitRangeChangedFiles( return parseChangedFilesAndNumStat( result.combinedOutput, - `${oldestCommitRef}..${shas[0]}` + `${oldestCommitRef}..${latestCommitRef}` ) } From 113c85bb3b94aad98748948f9cbbbf560cd1bea9 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 26 May 2022 11:27:50 -0400 Subject: [PATCH 33/84] Open up app-store diff method for multi commit --- app/src/lib/stores/app-store.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index c26b526475..4c0fc55ef8 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -1458,13 +1458,12 @@ export class AppStore extends TypedBaseStore { } } - // We do not get a diff when multiple commits selected - if (shas.length > 1) { + if (shas.length > 1 && !enableMultiCommitDiffs()) { return } const diff = - enableMultiCommitDiffs() && shas.length > 1 + shas.length > 1 ? await getCommitRangeDiff( repository, file, @@ -1481,9 +1480,13 @@ export class AppStore extends TypedBaseStore { const stateAfterLoad = this.repositoryStateCache.get(repository) const { shas: shasAfter } = stateAfterLoad.commitSelection // A whole bunch of things could have happened since we initiated the diff load - if (shasAfter.length !== shas.length || shasAfter[0] !== shas[0]) { + if ( + shasAfter.length !== shas.length || + !shas.every((sha, i) => sha === shasAfter[i]) + ) { return } + if (!stateAfterLoad.commitSelection.file) { return } From 73c332135d5084dd607995b5c48d33fc92e16dbd Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 26 May 2022 12:20:26 -0400 Subject: [PATCH 34/84] Refactor SelectedCommit to be SelectedCommits for multi commit diffing --- app/src/ui/history/index.ts | 2 +- app/src/ui/history/selected-commit.tsx | 73 ++++++++++++++------------ app/src/ui/repository.tsx | 30 +++++------ 3 files changed, 52 insertions(+), 53 deletions(-) diff --git a/app/src/ui/history/index.ts b/app/src/ui/history/index.ts index 5314aa21a1..7603ced06a 100644 --- a/app/src/ui/history/index.ts +++ b/app/src/ui/history/index.ts @@ -1,2 +1,2 @@ -export { SelectedCommit } from './selected-commit' +export { SelectedCommits } from './selected-commit' export { CompareSidebar } from './compare' diff --git a/app/src/ui/history/selected-commit.tsx b/app/src/ui/history/selected-commit.tsx index 9b004a0538..7559165b76 100644 --- a/app/src/ui/history/selected-commit.tsx +++ b/app/src/ui/history/selected-commit.tsx @@ -34,14 +34,15 @@ import { IChangesetData } from '../../lib/git' import { IConstrainedValue } from '../../lib/app-state' import { clamp } from '../../lib/clamp' import { pathExists } from '../lib/path-exists' +import { enableMultiCommitDiffs } from '../../lib/feature-flag' -interface ISelectedCommitProps { +interface ISelectedCommitsProps { readonly repository: Repository readonly isLocalRepository: boolean readonly dispatcher: Dispatcher readonly emoji: Map - readonly selectedCommit: Commit | null - readonly isLocal: boolean + readonly selectedCommits: ReadonlyArray + readonly localCommitSHAs: ReadonlyArray readonly changesetData: IChangesetData readonly selectedFile: CommittedFileChange | null readonly currentDiff: IDiff | null @@ -77,27 +78,24 @@ interface ISelectedCommitProps { /** Called when the user opens the diff options popover */ readonly onDiffOptionsOpened: () => void - /** Whether multiple commits are selected. */ - readonly areMultipleCommitsSelected: boolean - /** Whether or not to show the drag overlay */ readonly showDragOverlay: boolean } -interface ISelectedCommitState { +interface ISelectedCommitsState { readonly isExpanded: boolean readonly hideDescriptionBorder: boolean } /** The History component. Contains the commit list, commit summary, and diff. */ -export class SelectedCommit extends React.Component< - ISelectedCommitProps, - ISelectedCommitState +export class SelectedCommits extends React.Component< + ISelectedCommitsProps, + ISelectedCommitsState > { private readonly loadChangedFilesScheduler = new ThrottledScheduler(200) private historyRef: HTMLDivElement | null = null - public constructor(props: ISelectedCommitProps) { + public constructor(props: ISelectedCommitsProps) { super(props) this.state = { @@ -114,16 +112,12 @@ export class SelectedCommit extends React.Component< this.historyRef = ref } - public componentWillUpdate(nextProps: ISelectedCommitProps) { + public componentWillUpdate(nextProps: ISelectedCommitsProps) { // reset isExpanded if we're switching commits. - const currentValue = this.props.selectedCommit - ? this.props.selectedCommit.sha - : undefined - const nextValue = nextProps.selectedCommit - ? nextProps.selectedCommit.sha - : undefined + const currentValue = this.props.selectedCommits.join('') + const nextValue = nextProps.selectedCommits.join('') - if ((currentValue || nextValue) && currentValue !== nextValue) { + if (currentValue !== nextValue) { if (this.state.isExpanded) { this.setState({ isExpanded: false }) } @@ -250,13 +244,13 @@ export class SelectedCommit extends React.Component< } public render() { - const commit = this.props.selectedCommit + const { selectedCommits } = this.props - if (this.props.areMultipleCommitsSelected) { + if (selectedCommits.length > 1 && !enableMultiCommitDiffs()) { return this.renderMultipleCommitsSelected() } - if (commit == null) { + if (selectedCommits.length === 0) { return } @@ -265,7 +259,8 @@ export class SelectedCommit extends React.Component< return (
- {this.renderCommitSummary(commit)} + {selectedCommits.length === 1 && + this.renderCommitSummary(selectedCommits[0])}
{ event.preventDefault() - const fullPath = Path.join(this.props.repository.path, file.path) + const { + selectedCommits, + localCommitSHAs, + repository, + externalEditorLabel, + } = this.props + + const fullPath = Path.join(repository.path, file.path) const fileExistsOnDisk = await pathExists(fullPath) if (!fileExistsOnDisk) { showContextualMenu([ @@ -339,14 +341,14 @@ export class SelectedCommit extends React.Component< const extension = Path.extname(file.path) const isSafeExtension = isSafeFileExtension(extension) - const openInExternalEditor = this.props.externalEditorLabel - ? `Open in ${this.props.externalEditorLabel}` + const openInExternalEditor = externalEditorLabel + ? `Open in ${externalEditorLabel}` : DefaultEditorLabel const items: IMenuItem[] = [ { label: RevealInFileManagerLabel, - action: () => revealInFileManager(this.props.repository, file.path), + action: () => revealInFileManager(repository, file.path), enabled: fileExistsOnDisk, }, { @@ -372,7 +374,7 @@ export class SelectedCommit extends React.Component< ] let viewOnGitHubLabel = 'View on GitHub' - const gitHubRepository = this.props.repository.gitHubRepository + const gitHubRepository = repository.gitHubRepository if ( gitHubRepository && @@ -383,20 +385,21 @@ export class SelectedCommit extends React.Component< items.push({ label: viewOnGitHubLabel, - action: () => this.onViewOnGitHub(file), + action: () => this.onViewOnGitHub(selectedCommits[0].sha, file), enabled: - !this.props.isLocal && + // TODO: maybe we can assume to use last commit in multi commit + // scenario? + selectedCommits.length === 1 && + !localCommitSHAs.includes(selectedCommits[0].sha) && !!gitHubRepository && - !!this.props.selectedCommit, + this.props.selectedCommits.length > 0, }) showContextualMenu(items) } - private onViewOnGitHub = (file: CommittedFileChange) => { - if (this.props.selectedCommit && this.props.onViewCommitOnGitHub) { - this.props.onViewCommitOnGitHub(this.props.selectedCommit.sha, file.path) - } + private onViewOnGitHub = (sha: string, file: CommittedFileChange) => { + this.props.onViewCommitOnGitHub(sha, file.path) } } diff --git a/app/src/ui/repository.tsx b/app/src/ui/repository.tsx index c47874c6bf..b5fbf0001d 100644 --- a/app/src/ui/repository.tsx +++ b/app/src/ui/repository.tsx @@ -7,7 +7,7 @@ import { Changes, ChangesSidebar } from './changes' import { NoChanges } from './changes/no-changes' import { MultipleSelection } from './changes/multiple-selection' import { FilesChangedBadge } from './changes/files-changed-badge' -import { SelectedCommit, CompareSidebar } from './history' +import { SelectedCommits, CompareSidebar } from './history' import { Resizable } from './resizable' import { TabBar } from './tab-bar' import { @@ -372,31 +372,28 @@ export class RepositoryView extends React.Component< } private renderContentForHistory(): JSX.Element { - const { commitSelection } = this.props.state + const { commitSelection, commitLookup, localCommitSHAs } = this.props.state + const { changesetData, file, diff, shas } = commitSelection - const sha = - commitSelection.shas.length === 1 ? commitSelection.shas[0] : null - - const selectedCommit = - sha != null ? this.props.state.commitLookup.get(sha) || null : null - - const isLocal = - selectedCommit != null && - this.props.state.localCommitSHAs.includes(selectedCommit.sha) - - const { changesetData, file, diff } = commitSelection + const selectedCommits = [] + for (const sha of shas) { + const commit = commitLookup.get(sha) + if (commit !== undefined) { + selectedCommits.push(commit) + } + } const showDragOverlay = dragAndDropManager.isDragOfTypeInProgress( DragType.Commit ) return ( - 1} showDragOverlay={showDragOverlay} /> ) From 9ff8066607708f4d26bc15d442a52ea215233e8a Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 26 May 2022 14:00:31 -0400 Subject: [PATCH 35/84] Fix bug where switching branches didn't update first sha --- app/src/lib/stores/app-store.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 4c0fc55ef8..9e44aa9030 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -1108,9 +1108,10 @@ export class AppStore extends TypedBaseStore { ) { const state = this.repositoryStateCache.get(repository) let selectedSHA = - state.commitSelection.shas.length === 1 + state.commitSelection.shas.length > 0 ? state.commitSelection.shas[0] : null + if (selectedSHA != null) { const index = commitSHAs.findIndex(sha => sha === selectedSHA) if (index < 0) { @@ -1121,7 +1122,7 @@ export class AppStore extends TypedBaseStore { } } - if (state.commitSelection.shas.length === 0 && commitSHAs.length > 0) { + if (selectedSHA === null && commitSHAs.length > 0) { this._changeCommitSelection(repository, [commitSHAs[0]]) this._loadChangedFilesForCurrentSelection(repository) } From bc1a2f60787734410ad58434bc0dd7350d3e90f9 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 26 May 2022 14:40:24 -0400 Subject: [PATCH 36/84] Remove comment --- app/src/ui/history/selected-commit.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/src/ui/history/selected-commit.tsx b/app/src/ui/history/selected-commit.tsx index 7559165b76..24c17fd684 100644 --- a/app/src/ui/history/selected-commit.tsx +++ b/app/src/ui/history/selected-commit.tsx @@ -387,8 +387,6 @@ export class SelectedCommits extends React.Component< label: viewOnGitHubLabel, action: () => this.onViewOnGitHub(selectedCommits[0].sha, file), enabled: - // TODO: maybe we can assume to use last commit in multi commit - // scenario? selectedCommits.length === 1 && !localCommitSHAs.includes(selectedCommits[0].sha) && !!gitHubRepository && From dfa6c3d7bc5f9b572bde2d669039aa2575d30d66 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 26 May 2022 15:06:53 -0400 Subject: [PATCH 37/84] Refactor commit attribution to intake array of commits --- app/src/models/commit-identity.ts | 4 +++ app/src/ui/history/commit-list-item.tsx | 2 +- app/src/ui/history/commit-summary.tsx | 2 +- app/src/ui/lib/commit-attribution.tsx | 40 +++++++++++++++---------- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/app/src/models/commit-identity.ts b/app/src/models/commit-identity.ts index d5c9246848..d66d681631 100644 --- a/app/src/models/commit-identity.ts +++ b/app/src/models/commit-identity.ts @@ -55,4 +55,8 @@ export class CommitIdentity { public readonly date: Date, public readonly tzOffset: number = new Date().getTimezoneOffset() ) {} + + public toString() { + return `${this.name} <${this.email}>` + } } diff --git a/app/src/ui/history/commit-list-item.tsx b/app/src/ui/history/commit-list-item.tsx index c93a14af26..374a299173 100644 --- a/app/src/ui/history/commit-list-item.tsx +++ b/app/src/ui/history/commit-list-item.tsx @@ -174,7 +174,7 @@ export class CommitListItem extends React.PureComponent<
{renderRelativeTime(date)}
diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index 8eef26f5b6..d554e10f86 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -330,7 +330,7 @@ export class CommitSummary extends React.Component< diff --git a/app/src/ui/lib/commit-attribution.tsx b/app/src/ui/lib/commit-attribution.tsx index 2a7c775b94..9d6bdf3a87 100644 --- a/app/src/ui/lib/commit-attribution.tsx +++ b/app/src/ui/lib/commit-attribution.tsx @@ -7,10 +7,10 @@ import { isWebFlowCommitter } from '../../lib/web-flow-committer' interface ICommitAttributionProps { /** - * The commit from where to extract the author, committer + * The commit or commits from where to extract the author, committer * and co-authors from. */ - readonly commit: Commit + readonly commits: ReadonlyArray /** * The GitHub hosted repository that the given commit is @@ -61,24 +61,34 @@ export class CommitAttribution extends React.Component< } public render() { - const commit = this.props.commit - const { author, committer, coAuthors } = commit + const { commits } = this.props - // do we need to attribute the committer separately from the author? - const committerAttribution = - !commit.authoredByCommitter && - !( - this.props.gitHubRepository !== null && - isWebFlowCommitter(commit, this.props.gitHubRepository) - ) + const allAuthors = new Map() + for (const commit of commits) { + const { author, committer, coAuthors } = commit - const authors: Array = committerAttribution - ? [author, committer, ...coAuthors] - : [author, ...coAuthors] + // do we need to attribute the committer separately from the author? + const committerAttribution = + !commit.authoredByCommitter && + !( + this.props.gitHubRepository !== null && + isWebFlowCommitter(commit, this.props.gitHubRepository) + ) + + const authors: Array = committerAttribution + ? [author, committer, ...coAuthors] + : [author, ...coAuthors] + + for (const a of authors) { + if (allAuthors.get(a.toString()) === undefined) { + allAuthors.set(a.toString(), a) + } + } + } return ( - {this.renderAuthors(authors)} + {this.renderAuthors(Array.from(allAuthors.values()))} ) } From 00bcdd5c0c586418fec52164e9b6f6ac61582a38 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 26 May 2022 15:36:44 -0400 Subject: [PATCH 38/84] Refactor Commit Summary to accept array of commits Avatar user logic handles array --- app/src/ui/history/commit-summary.tsx | 28 +++++++++++++++----------- app/src/ui/history/selected-commit.tsx | 7 +++---- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index d554e10f86..3d8efc1209 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -18,10 +18,11 @@ import { TooltippedContent } from '../lib/tooltipped-content' import { clipboard } from 'electron' import { TooltipDirection } from '../lib/tooltip' import { AppFileStatusKind } from '../../models/status' +import _ from 'lodash' interface ICommitSummaryProps { readonly repository: Repository - readonly commit: Commit + readonly commits: ReadonlyArray readonly changesetData: IChangesetData readonly emoji: Map @@ -101,14 +102,17 @@ function createState( const tokenizer = new Tokenizer(props.emoji, props.repository) const { summary, body } = wrapRichTextCommitMessage( - props.commit.summary, - props.commit.body, + props.commits[0].summary, + props.commits[0].body, tokenizer ) - const avatarUsers = getAvatarUsersForCommit( - props.repository.gitHubRepository, - props.commit + const allAvatarUsers = props.commits.flatMap(c => + getAvatarUsersForCommit(props.repository.gitHubRepository, c) + ) + const avatarUsers = _.uniqWith( + allAvatarUsers, + (a, b) => a.email === b.email && a.name === b.name ) return { isOverflowed, summary, body, avatarUsers } @@ -242,7 +246,7 @@ export class CommitSummary extends React.Component< } public componentWillUpdate(nextProps: ICommitSummaryProps) { - if (!messageEquals(nextProps.commit, this.props.commit)) { + if (!messageEquals(nextProps.commits[0], this.props.commits[0])) { this.setState(createState(false, nextProps)) } } @@ -294,7 +298,7 @@ export class CommitSummary extends React.Component< } public render() { - const shortSHA = this.props.commit.shortSha + const shortSHA = this.props.commits[0].shortSha const className = classNames({ expanded: this.props.isExpanded, @@ -330,7 +334,7 @@ export class CommitSummary extends React.Component< @@ -382,7 +386,7 @@ export class CommitSummary extends React.Component< private renderShaTooltip() { return ( <> - {this.props.commit.sha} + {this.props.commits[0].sha} ) @@ -390,7 +394,7 @@ export class CommitSummary extends React.Component< private onCopyShaButtonClick = (e: React.MouseEvent) => { e.preventDefault() - clipboard.writeText(this.props.commit.sha) + clipboard.writeText(this.props.commits[0].sha) } private renderChangedFilesDescription = () => { @@ -492,7 +496,7 @@ export class CommitSummary extends React.Component< } private renderTags() { - const tags = this.props.commit.tags || [] + const tags = this.props.commits[0].tags || [] if (tags.length === 0) { return null diff --git a/app/src/ui/history/selected-commit.tsx b/app/src/ui/history/selected-commit.tsx index 24c17fd684..a9b86c871f 100644 --- a/app/src/ui/history/selected-commit.tsx +++ b/app/src/ui/history/selected-commit.tsx @@ -160,10 +160,10 @@ export class SelectedCommits extends React.Component< ) } - private renderCommitSummary(commit: Commit) { + private renderCommitSummary(commits: ReadonlyArray) { return ( - {selectedCommits.length === 1 && - this.renderCommitSummary(selectedCommits[0])} + {this.renderCommitSummary(selectedCommits)}
Date: Thu, 26 May 2022 15:44:56 -0400 Subject: [PATCH 39/84] Commit summary tags accepts multiple commits --- app/src/ui/history/commit-summary.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index 3d8efc1209..caf8ca0d36 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -496,7 +496,7 @@ export class CommitSummary extends React.Component< } private renderTags() { - const tags = this.props.commits[0].tags || [] + const tags = this.props.commits.flatMap(c => c.tags) || [] if (tags.length === 0) { return null From c751cb9cf817d06da1308f51ab93b844b2b72d24 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 26 May 2022 15:56:43 -0400 Subject: [PATCH 40/84] Handles multi commit sha ref --- app/src/ui/history/commit-summary.tsx | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index caf8ca0d36..b5f9c7583a 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -297,9 +297,20 @@ export class CommitSummary extends React.Component< ) } - public render() { - const shortSHA = this.props.commits[0].shortSha + private getShaRef = (useShortSha?: boolean) => { + const { commits } = this.props + const oldest = useShortSha ? commits[0].shortSha : commits[0].sha + if (commits.length === 1) { + return oldest + } + + const latest = useShortSha ? commits.at(-1)?.shortSha : commits.at(-1)?.sha + + return `${oldest}^..${latest}` + } + + public render() { const className = classNames({ expanded: this.props.isExpanded, collapsed: !this.props.isExpanded, @@ -350,7 +361,7 @@ export class CommitSummary extends React.Component< interactive={true} direction={TooltipDirection.SOUTH} > - {shortSHA} + {this.getShaRef(true)} @@ -386,7 +397,7 @@ export class CommitSummary extends React.Component< private renderShaTooltip() { return ( <> - {this.props.commits[0].sha} + {this.getShaRef()} ) @@ -394,7 +405,7 @@ export class CommitSummary extends React.Component< private onCopyShaButtonClick = (e: React.MouseEvent) => { e.preventDefault() - clipboard.writeText(this.props.commits[0].sha) + clipboard.writeText(this.getShaRef()) } private renderChangedFilesDescription = () => { From e5d39d6a68bc923f49b5442d5541c8a4a54b123d Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Fri, 27 May 2022 08:57:57 -0400 Subject: [PATCH 41/84] Avatar need more space for > 3 avatars --- app/styles/ui/history/_commit-summary.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/styles/ui/history/_commit-summary.scss b/app/styles/ui/history/_commit-summary.scss index 3b5b31ac84..43b6fdbe7f 100644 --- a/app/styles/ui/history/_commit-summary.scss +++ b/app/styles/ui/history/_commit-summary.scss @@ -11,6 +11,10 @@ height: 16px; } + .AvatarStack.AvatarStack--small.AvatarStack--three-plus { + min-width: 46px; + } + .expander { position: absolute; width: 75px; From 82faac29b37345d6bfa05841a86fe1bef172ed6b Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Fri, 27 May 2022 08:58:10 -0400 Subject: [PATCH 42/84] Multiple commit header --- app/src/ui/history/commit-summary.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index b5f9c7583a..bfd5164936 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -321,6 +321,8 @@ export class CommitSummary extends React.Component< 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', { @@ -389,7 +391,7 @@ export class CommitSummary extends React.Component<
- {this.renderDescription()} + {this.props.commits.length === 1 && this.renderDescription()}
) } From 8078c531c4b63a1d06afe757f0d4c09058682aae Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Fri, 27 May 2022 09:22:39 -0400 Subject: [PATCH 43/84] Update change check to account for multiple commits --- app/src/ui/history/commit-summary.tsx | 7 ++++++- app/src/ui/history/selected-commit.tsx | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index bfd5164936..502d05de1d 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -246,7 +246,12 @@ export class CommitSummary extends React.Component< } public componentWillUpdate(nextProps: ICommitSummaryProps) { - if (!messageEquals(nextProps.commits[0], this.props.commits[0])) { + if ( + nextProps.commits.length !== this.props.commits.length || + !nextProps.commits.every((nextCommit, i) => + messageEquals(nextCommit, this.props.commits[i]) + ) + ) { this.setState(createState(false, nextProps)) } } diff --git a/app/src/ui/history/selected-commit.tsx b/app/src/ui/history/selected-commit.tsx index a9b86c871f..d6fea93e2b 100644 --- a/app/src/ui/history/selected-commit.tsx +++ b/app/src/ui/history/selected-commit.tsx @@ -114,8 +114,8 @@ export class SelectedCommits extends React.Component< public componentWillUpdate(nextProps: ISelectedCommitsProps) { // reset isExpanded if we're switching commits. - const currentValue = this.props.selectedCommits.join('') - const nextValue = nextProps.selectedCommits.join('') + const currentValue = this.props.selectedCommits.map(c => c.sha).join('') + const nextValue = nextProps.selectedCommits.map(c => c.sha).join('') if (currentValue !== nextValue) { if (this.state.isExpanded) { From 8b7cdac4e06c1b6d6755bcb4ace0366dce09c1d8 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Fri, 27 May 2022 09:23:09 -0400 Subject: [PATCH 44/84] Build multi commit description --- app/src/ui/history/commit-summary.tsx | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index 502d05de1d..7f21b5e56f 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -99,16 +99,29 @@ function createState( isOverflowed: boolean, props: ICommitSummaryProps ): ICommitSummaryState { - const tokenizer = new Tokenizer(props.emoji, props.repository) + const { emoji, repository, commits } = 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( - props.commits[0].summary, - props.commits[0].body, + commits[0].summary, + plainTextBody, tokenizer ) - const allAvatarUsers = props.commits.flatMap(c => - getAvatarUsersForCommit(props.repository.gitHubRepository, c) + const allAvatarUsers = commits.flatMap(c => + getAvatarUsersForCommit(repository.gitHubRepository, c) ) const avatarUsers = _.uniqWith( allAvatarUsers, @@ -396,7 +409,7 @@ export class CommitSummary extends React.Component<
- {this.props.commits.length === 1 && this.renderDescription()} + {this.renderDescription()} ) } From bf0168ccc6657db84117e11ea19a2be493d67409 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Fri, 27 May 2022 09:33:39 -0400 Subject: [PATCH 45/84] Give it all the authors.. --- app/src/ui/history/commit-summary.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index 7f21b5e56f..50998686e9 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -365,7 +365,7 @@ export class CommitSummary extends React.Component< From 635a0d77f7f94402893b33310f8ac8ac98876346 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Fri, 27 May 2022 09:45:57 -0400 Subject: [PATCH 46/84] Fix avatar spacing --- app/src/ui/lib/avatar-stack.tsx | 3 ++- app/styles/ui/_avatar-stack.scss | 13 +++++++++++-- app/styles/ui/history/_commit-summary.scss | 4 ---- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/app/src/ui/lib/avatar-stack.tsx b/app/src/ui/lib/avatar-stack.tsx index 4b3d3da4b6..f725cee0c9 100644 --- a/app/src/ui/lib/avatar-stack.tsx +++ b/app/src/ui/lib/avatar-stack.tsx @@ -35,7 +35,8 @@ export class AvatarStack extends React.Component { const className = classNames('AvatarStack', { 'AvatarStack--small': true, 'AvatarStack--two': users.length === 2, - 'AvatarStack--three-plus': users.length >= MaxDisplayedAvatars, + 'AvatarStack--three': users.length === 3, + 'AvatarStack--plus': users.length > MaxDisplayedAvatars, }) return ( diff --git a/app/styles/ui/_avatar-stack.scss b/app/styles/ui/_avatar-stack.scss index 1b79570ead..080fccbb92 100644 --- a/app/styles/ui/_avatar-stack.scss +++ b/app/styles/ui/_avatar-stack.scss @@ -11,7 +11,11 @@ min-width: 36px; } - &.AvatarStack--three-plus { + &.AvatarStack--three { + min-width: 30px; + } + + &.AvatarStack--plus { min-width: 46px; } @@ -28,10 +32,14 @@ min-width: 25px; } - &.AvatarStack--three-plus { + &.AvatarStack--three { min-width: 30px; } + &.AvatarStack--plus { + min-width: 46px; + } + .avatar.avatar-more { &::before, &::after { @@ -121,6 +129,7 @@ z-index: 1; margin-right: 0; background: $gray-100; + width: 10px !important; &::before, &::after { diff --git a/app/styles/ui/history/_commit-summary.scss b/app/styles/ui/history/_commit-summary.scss index 43b6fdbe7f..3b5b31ac84 100644 --- a/app/styles/ui/history/_commit-summary.scss +++ b/app/styles/ui/history/_commit-summary.scss @@ -11,10 +11,6 @@ height: 16px; } - .AvatarStack.AvatarStack--small.AvatarStack--three-plus { - min-width: 46px; - } - .expander { position: absolute; width: 75px; From 105103891cf68c5d02a66791ac9b90e3af6219e9 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 31 May 2022 09:42:30 -0400 Subject: [PATCH 47/84] Description can be very very long.. --- app/styles/ui/history/_commit-summary.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/styles/ui/history/_commit-summary.scss b/app/styles/ui/history/_commit-summary.scss index 3b5b31ac84..704ba0762e 100644 --- a/app/styles/ui/history/_commit-summary.scss +++ b/app/styles/ui/history/_commit-summary.scss @@ -25,7 +25,7 @@ &.expanded { .commit-summary-description-scroll-view { - max-height: none; + max-height: 400px; overflow: auto; &:before { From 5ad4d68901988a1c94933bc65e28ddf5da4afc2c Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Tue, 31 May 2022 11:33:02 -0400 Subject: [PATCH 48/84] Improved readability of latestCommit Co-authored-by: Sergio Padrino --- app/src/ui/history/commit-summary.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index 50998686e9..f16c83a66a 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -323,7 +323,8 @@ export class CommitSummary extends React.Component< return oldest } - const latest = useShortSha ? commits.at(-1)?.shortSha : commits.at(-1)?.sha + const latestCommit = commits.at(-1) + const latest = useShortSha ? latestCommit?.shortSha : latestCommit?.sha return `${oldest}^..${latest}` } From 97c1029841f6bc8ad83e501558477bec576f15ba Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Tue, 31 May 2022 11:33:32 -0400 Subject: [PATCH 49/84] Has of get check Co-authored-by: Sergio Padrino --- app/src/ui/lib/commit-attribution.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/ui/lib/commit-attribution.tsx b/app/src/ui/lib/commit-attribution.tsx index 9d6bdf3a87..d0fbc40980 100644 --- a/app/src/ui/lib/commit-attribution.tsx +++ b/app/src/ui/lib/commit-attribution.tsx @@ -80,7 +80,7 @@ export class CommitAttribution extends React.Component< : [author, ...coAuthors] for (const a of authors) { - if (allAuthors.get(a.toString()) === undefined) { + if (!allAuthors.has(a.toString())) { allAuthors.set(a.toString(), a) } } From a041b320763352e3716f0f3d4591c41cc9bc69d3 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 31 May 2022 11:45:02 -0400 Subject: [PATCH 50/84] Avatars of 4+ all take same space --- app/styles/ui/_avatar-stack.scss | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/app/styles/ui/_avatar-stack.scss b/app/styles/ui/_avatar-stack.scss index 080fccbb92..52bfedf5d9 100644 --- a/app/styles/ui/_avatar-stack.scss +++ b/app/styles/ui/_avatar-stack.scss @@ -37,7 +37,7 @@ } &.AvatarStack--plus { - min-width: 46px; + min-width: 40px; } .avatar.avatar-more { @@ -74,6 +74,13 @@ background: var(--box-alt-background-color); } + .avatar-container:nth-child(n + 5) { + .avatar { + display: none; + opacity: 0; + } + } + .avatar { position: relative; z-index: 2; @@ -100,13 +107,6 @@ img { border-radius: 50%; } - // stylelint-enable selector-max-type - - // Account for 4+ avatars - &:nth-child(n + 4) { - display: none; - opacity: 0; - } } &:hover { @@ -114,9 +114,11 @@ margin-right: 3px; } - .avatar:nth-child(n + 4) { - display: flex; - opacity: 1; + .avatar-container:nth-child(n + 5) { + .avatar { + display: flex; + opacity: 1; + } } .avatar-more { From 4baf2a44b47a49eb4b1b6b89ba13b8f2101d0e74 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 31 May 2022 12:03:04 -0400 Subject: [PATCH 51/84] More symbol at 5 and more users. --- app/src/ui/lib/avatar-stack.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/src/ui/lib/avatar-stack.tsx b/app/src/ui/lib/avatar-stack.tsx index f725cee0c9..f0681292ef 100644 --- a/app/src/ui/lib/avatar-stack.tsx +++ b/app/src/ui/lib/avatar-stack.tsx @@ -25,7 +25,10 @@ export class AvatarStack extends React.Component { const users = this.props.users for (let i = 0; i < this.props.users.length; i++) { - if (users.length > MaxDisplayedAvatars && i === MaxDisplayedAvatars - 1) { + if ( + users.length > MaxDisplayedAvatars + 1 && + i === MaxDisplayedAvatars - 1 + ) { elems.push(
) } From 663033389d658a3c229f8e9754ce552709a6cfb2 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 31 May 2022 14:05:07 -0400 Subject: [PATCH 52/84] Don't diff if not contiguous --- app/src/lib/app-state.ts | 14 ++++++++ app/src/lib/stores/app-store.ts | 14 ++++---- app/src/lib/stores/repository-state-cache.ts | 1 + app/src/ui/dispatcher/dispatcher.ts | 13 +++++--- app/src/ui/history/commit-list.tsx | 35 +++++++++++++++++--- app/src/ui/history/compare.tsx | 8 +++-- 6 files changed, 69 insertions(+), 16 deletions(-) diff --git a/app/src/lib/app-state.ts b/app/src/lib/app-state.ts index 4f6e6233c9..d02694c324 100644 --- a/app/src/lib/app-state.ts +++ b/app/src/lib/app-state.ts @@ -553,6 +553,20 @@ export interface ICommitSelection { /** The commits currently selected in the app */ readonly shas: 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 + * opposed to 3, 5, 8. + * + * Technically order does not matter, but shas are stored in order. + * + * Contiguous selections can be diffed. Non-contiguous selections can be + * cherry-picked, reordered, or squashed. + * + * Assumed that a selections of zero or one commit are contiguous. + * */ + readonly isContiguous: boolean + /** The changeset data associated with the selected commit */ readonly changesetData: IChangesetData diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 9e44aa9030..301b7974c8 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -1081,7 +1081,8 @@ export class AppStore extends TypedBaseStore { /** This shouldn't be called directly. See `Dispatcher`. */ public async _changeCommitSelection( repository: Repository, - shas: ReadonlyArray + shas: ReadonlyArray, + isContiguous: boolean ): Promise { const { commitSelection } = this.repositoryStateCache.get(repository) @@ -1094,6 +1095,7 @@ export class AppStore extends TypedBaseStore { this.repositoryStateCache.updateCommitSelection(repository, () => ({ shas, + isContiguous, file: null, changesetData: { files: [], linesAdded: 0, linesDeleted: 0 }, diff: null, @@ -1123,7 +1125,7 @@ export class AppStore extends TypedBaseStore { } if (selectedSHA === null && commitSHAs.length > 0) { - this._changeCommitSelection(repository, [commitSHAs[0]]) + this._changeCommitSelection(repository, [commitSHAs[0]], true) this._loadChangedFilesForCurrentSelection(repository) } } @@ -1378,10 +1380,10 @@ export class AppStore extends TypedBaseStore { ): Promise { const state = this.repositoryStateCache.get(repository) const { commitSelection } = state - const currentSHAs = commitSelection.shas + const { shas: currentSHAs, isContiguous } = commitSelection if ( currentSHAs.length === 0 || - (currentSHAs.length !== 1 && !enableMultiCommitDiffs()) + (currentSHAs.length > 1 && (!enableMultiCommitDiffs() || !isContiguous)) ) { return } @@ -1447,7 +1449,7 @@ export class AppStore extends TypedBaseStore { this.emitUpdate() const stateBeforeLoad = this.repositoryStateCache.get(repository) - const shas = stateBeforeLoad.commitSelection.shas + const { shas, isContiguous } = stateBeforeLoad.commitSelection if (shas.length === 0) { if (__DEV__) { @@ -1459,7 +1461,7 @@ export class AppStore extends TypedBaseStore { } } - if (shas.length > 1 && !enableMultiCommitDiffs()) { + if (shas.length > 1 && (!enableMultiCommitDiffs() || !isContiguous)) { return } diff --git a/app/src/lib/stores/repository-state-cache.ts b/app/src/lib/stores/repository-state-cache.ts index c1cb5f1770..d7e2586657 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: [], + isContiguous: true, file: null, changesetData: { files: [], linesAdded: 0, linesDeleted: 0 }, diff: null, diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index 4d86966124..9e7630e177 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -235,9 +235,10 @@ export class Dispatcher { */ public changeCommitSelection( repository: Repository, - shas: ReadonlyArray + shas: ReadonlyArray, + isContiguous: boolean ): Promise { - return this.appStore._changeCommitSelection(repository, shas) + return this.appStore._changeCommitSelection(repository, shas, isContiguous) } /** @@ -3148,7 +3149,7 @@ export class Dispatcher { switch (cherryPickResult) { case CherryPickResult.CompletedWithoutError: - await this.changeCommitSelection(repository, [commits[0].sha]) + await this.changeCommitSelection(repository, [commits[0].sha], true) await this.completeMultiCommitOperation(repository, commits.length) break case CherryPickResult.ConflictsEncountered: @@ -3552,7 +3553,11 @@ export class Dispatcher { // TODO: Look at history back to last retained commit and search for // squashed commit based on new commit message ... if there is more // than one, just take the most recent. (not likely?) - await this.changeCommitSelection(repository, [status.currentTip]) + await this.changeCommitSelection( + repository, + [status.currentTip], + true + ) } await this.completeMultiCommitOperation( diff --git a/app/src/ui/history/commit-list.tsx b/app/src/ui/history/commit-list.tsx index af78a79d4e..3943ed78ee 100644 --- a/app/src/ui/history/commit-list.tsx +++ b/app/src/ui/history/commit-list.tsx @@ -41,7 +41,10 @@ interface ICommitListProps { readonly emptyListMessage: JSX.Element | string /** Callback which fires when a commit has been selected in the list */ - readonly onCommitsSelected: (commits: ReadonlyArray) => void + readonly onCommitsSelected: ( + commits: ReadonlyArray, + isContiguous: boolean + ) => void /** Callback that fires when a scroll event has occurred */ readonly onScroll: (start: number, end: number) => void @@ -269,10 +272,34 @@ export class CommitList extends React.Component { // reordering, they will need to do multiple cherry-picks. // Goal: first commit in history -> first on array 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.props.onCommitsSelected(selectedCommits, this.isContiguous(sorted)) + } + + /** + * Accepts a sorted array of numbers in descending order. If the numbers ar + * contiguous order, 4, 3, 2 not 5, 3, 1, returns true. + * + * Defined an array of 0 and 1 are considered contiguous. + */ + private isContiguous(indexes: ReadonlyArray) { + if (indexes.length <= 1) { + return true + } + + for (let i = 0; i < indexes.length; i++) { + const current = indexes[i] + if (i + 1 === indexes.length) { + continue + } + + if (current - 1 !== indexes[i + 1]) { + return false + } + } + + return true } // This is required along with onSelectedRangeChanged in the case of a user @@ -281,7 +308,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]) + this.props.onCommitsSelected([commit], true) } } diff --git a/app/src/ui/history/compare.tsx b/app/src/ui/history/compare.tsx index c2fb522e29..7d74d46edd 100644 --- a/app/src/ui/history/compare.tsx +++ b/app/src/ui/history/compare.tsx @@ -460,10 +460,14 @@ export class CompareSidebar extends React.Component< } } - private onCommitsSelected = (commits: ReadonlyArray) => { + private onCommitsSelected = ( + commits: ReadonlyArray, + isContiguous: boolean + ) => { this.props.dispatcher.changeCommitSelection( this.props.repository, - commits.map(c => c.sha) + commits.map(c => c.sha), + isContiguous ) this.loadChangedFilesScheduler.queue(() => { From 4c45360a6caf2ed0cb519ef29ad70b537160d0ac Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 31 May 2022 14:20:08 -0400 Subject: [PATCH 53/84] Show multiple commit blank slate for non-contiguous selections --- app/src/ui/history/selected-commit.tsx | 27 ++++++++++++++++++++------ app/src/ui/repository.tsx | 3 ++- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/app/src/ui/history/selected-commit.tsx b/app/src/ui/history/selected-commit.tsx index d6fea93e2b..8b8e7cab13 100644 --- a/app/src/ui/history/selected-commit.tsx +++ b/app/src/ui/history/selected-commit.tsx @@ -80,6 +80,9 @@ interface ISelectedCommitsProps { /** Whether or not to show the drag overlay */ readonly showDragOverlay: boolean + + /** Whether or not the selection of commits is contiguous */ + readonly isContiguous: boolean } interface ISelectedCommitsState { @@ -244,10 +247,13 @@ export class SelectedCommits extends React.Component< } public render() { - const { selectedCommits } = this.props + const { selectedCommits, isContiguous } = this.props - if (selectedCommits.length > 1 && !enableMultiCommitDiffs()) { - return this.renderMultipleCommitsSelected() + if ( + selectedCommits.length > 1 && + (!isContiguous || !enableMultiCommitDiffs()) + ) { + return this.renderMultipleCommitsBlankSlate() } if (selectedCommits.length === 0) { @@ -285,7 +291,7 @@ export class SelectedCommits extends React.Component< return
} - private renderMultipleCommitsSelected(): JSX.Element { + private renderMultipleCommitsBlankSlate(): JSX.Element { const BlankSlateImage = encodePathAsUrl( __dirname, 'static/empty-no-commit.svg' @@ -296,11 +302,20 @@ export class SelectedCommits extends React.Component<
-

Unable to display diff when multiple commits are selected.

+

+ Unable to display diff when multiple{' '} + {enableMultiCommitDiffs() ? 'non-adjacent ' : ' '}commits are + selected. +

You can:
    -
  • Select a single commit to view a diff.
    • +
  • + Select a single commit{' '} + {enableMultiCommitDiffs() ? 'or a range of commits ' : ' '}to + view a diff. +
  • Drag the commits to the branch menu to cherry-pick them.
    • +
  • Drag the commits to squash or reorder them.
  • Right click on multiple commits to see options.
diff --git a/app/src/ui/repository.tsx b/app/src/ui/repository.tsx index b5fbf0001d..9d9ac732c0 100644 --- a/app/src/ui/repository.tsx +++ b/app/src/ui/repository.tsx @@ -373,7 +373,7 @@ export class RepositoryView extends React.Component< private renderContentForHistory(): JSX.Element { const { commitSelection, commitLookup, localCommitSHAs } = this.props.state - const { changesetData, file, diff, shas } = commitSelection + const { changesetData, file, diff, shas, isContiguous } = commitSelection const selectedCommits = [] for (const sha of shas) { @@ -393,6 +393,7 @@ export class RepositoryView extends React.Component< isLocalRepository={this.props.state.remote === null} dispatcher={this.props.dispatcher} selectedCommits={selectedCommits} + isContiguous={isContiguous} localCommitSHAs={localCommitSHAs} changesetData={changesetData} selectedFile={file} From 1395391d6378254a6020470db2f7b5a56abb1543 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Wed, 1 Jun 2022 11:54:18 +0200 Subject: [PATCH 54/84] Bump version and changelog to 3.0.2-beta1 --- app/package.json | 2 +- changelog.json | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/package.json b/app/package.json index dadc397241..f039b4099c 100644 --- a/app/package.json +++ b/app/package.json @@ -3,7 +3,7 @@ "productName": "GitHub Desktop", "bundleID": "com.github.GitHubClient", "companyName": "GitHub, Inc.", - "version": "3.0.1", + "version": "3.0.2-beta1", "main": "./main.js", "repository": { "type": "git", diff --git a/changelog.json b/changelog.json index d467808603..4002d7a2a0 100644 --- a/changelog.json +++ b/changelog.json @@ -1,5 +1,8 @@ { "releases": { + "3.0.2-beta1": [ + "[Added] Add support for Aptana Studio - #14669. Thanks @tsvetilian-ty!" + ], "3.0.1": [ "[Added] Add support for PyCharm Community Edition on Windows - #14411. Thanks @tsvetilian-ty!", "[Added] Add support for highlighting .mjs/.cjs/.mts/.cts files as JavaScript/TypeScript - #14481. Thanks @j-f1!", From 9e7b01373ae3a10bbb76ad2d8cdfd35bb7d73dbf Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Wed, 1 Jun 2022 13:40:07 +0200 Subject: [PATCH 55/84] Fix Markdown highlighting Add missing `lookAhead` function required by the Markdown mode --- app/src/highlighter/globals.d.ts | 1 + app/src/highlighter/index.ts | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/src/highlighter/globals.d.ts b/app/src/highlighter/globals.d.ts index f17b1b2349..bdefb21dfb 100644 --- a/app/src/highlighter/globals.d.ts +++ b/app/src/highlighter/globals.d.ts @@ -19,6 +19,7 @@ declare namespace CodeMirror { interface StringStreamContext { lines: string[] line: number + lookAhead: (n: number) => string } /** diff --git a/app/src/highlighter/index.ts b/app/src/highlighter/index.ts index 76fc1ddf0a..63885be640 100644 --- a/app/src/highlighter/index.ts +++ b/app/src/highlighter/index.ts @@ -635,7 +635,11 @@ onmessage = async (ev: MessageEvent) => { continue } - const lineCtx = { lines, line: ix } + const lineCtx = { + lines, + line: ix, + lookAhead: (n: number) => lines[ix + n], + } const lineStream = new StringStream(line, tabSize, lineCtx) while (!lineStream.eol()) { From e36beb1a68727c39a96d46fd813c67e269c0ab8b Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 1 Jun 2022 08:46:48 -0400 Subject: [PATCH 56/84] No shadowing --- app/src/lib/git/diff.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index 9b270fb5cc..f4f8794f50 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -181,13 +181,12 @@ export async function getCommitRangeDiff( // initial commit of a branch) and therefore `SHA^` is not a valid reference. // In which case, we will retry with the null tree sha. if (result.gitError === GitError.BadRevision && useNullTreeSHA === false) { - const useNullTreeSHA = true return getCommitRangeDiff( repository, file, commits, hideWhitespaceInDiff, - useNullTreeSHA + true ) } From ed004b9300fd0a211d1e434dc9cfa27c854581b6 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 1 Jun 2022 08:47:12 -0400 Subject: [PATCH 57/84] Import from core --- app/src/lib/git/diff.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index f4f8794f50..b62952219e 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -27,7 +27,7 @@ import { getOldPathOrDefault } from '../get-old-path' import { getCaptures } from '../helpers/regex' import { readFile } from 'fs/promises' import { forceUnwrap } from '../fatal-error' -import { git } from '.' +import { git } from './core' import { NullTreeSHA } from './diff-index' import { GitError } from 'dugite' From a2b33b729a9eb986d7ed071cfde3588d1cc1b550 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 2 Jun 2022 17:20:06 +0200 Subject: [PATCH 58/84] Bump desktop-notifications to 0.2.1 --- app/package.json | 2 +- app/yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/package.json b/app/package.json index f039b4099c..644a3a2fac 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.0", + "desktop-notifications": "^0.2.1", "desktop-trampoline": "desktop/desktop-trampoline#v0.9.8", "dexie": "^3.2.2", "dompurify": "^2.3.3", diff --git a/app/yarn.lock b/app/yarn.lock index 8e44d894eb..063142356c 100644 --- a/app/yarn.lock +++ b/app/yarn.lock @@ -363,10 +363,10 @@ delegates@^1.0.0: resolved "https://registry.yarnpkg.com/delegates/-/delegates-1.0.0.tgz#84c6e159b81904fdca59a0ef44cd870d31250f9a" integrity sha1-hMbhWbgZBP3KWaDvRM2HDTElD5o= -desktop-notifications@^0.2.0: - version "0.2.0" - resolved "https://registry.yarnpkg.com/desktop-notifications/-/desktop-notifications-0.2.0.tgz#8ec6d732671d42e9aa4dac4523a2a57806c9e210" - integrity sha512-6puOHRkSHmH1HpQDftFtpJU+ko7gw1vtSlf2LViJX+sY2N+9pVuYO85HjTlpktHDSVKvDnFGhPVAoZmjXIvo/Q== +desktop-notifications@^0.2.1: + version "0.2.1" + resolved "https://registry.yarnpkg.com/desktop-notifications/-/desktop-notifications-0.2.1.tgz#ff934e45ef6b47cfbbd6c596c759fc91b96ab753" + integrity sha512-ssB7SYsWlVgFCN/4KE5QIHM1hH4PXGHmlIYpUa4xR7WDHcjMEU6vrnrAFWx9SnPLVkCNDiWRwSDejlS1VF9BHw== dependencies: node-addon-api "^5.0.0" prebuild-install "^7.0.1" From ab4d7daa572768110ebd827d8dfd9c748dc3511c Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 3 Jun 2022 08:35:56 +0200 Subject: [PATCH 59/84] Bump changelog and version to 3.0.2-beta2 --- app/package.json | 2 +- changelog.json | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/package.json b/app/package.json index 644a3a2fac..6a7777745e 100644 --- a/app/package.json +++ b/app/package.json @@ -3,7 +3,7 @@ "productName": "GitHub Desktop", "bundleID": "com.github.GitHubClient", "companyName": "GitHub, Inc.", - "version": "3.0.2-beta1", + "version": "3.0.2-beta2", "main": "./main.js", "repository": { "type": "git", diff --git a/changelog.json b/changelog.json index 4002d7a2a0..142a1b26b6 100644 --- a/changelog.json +++ b/changelog.json @@ -1,5 +1,8 @@ { "releases": { + "3.0.2-beta2": [ + "[Fixed] Fix crash launching the app on macOS High Sierra - #14712" + ], "3.0.2-beta1": [ "[Added] Add support for Aptana Studio - #14669. Thanks @tsvetilian-ty!" ], From bf337818cf769b3580ecade6b75ef295bf3cda70 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 3 Jun 2022 14:06:53 +0200 Subject: [PATCH 60/84] Improve draft-release script to create release branch --- script/draft-release/run.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/script/draft-release/run.ts b/script/draft-release/run.ts index 9aa30be162..fdbfc4b706 100644 --- a/script/draft-release/run.ts +++ b/script/draft-release/run.ts @@ -51,6 +51,20 @@ async function getLatestRelease(options: { return latestTag instanceof SemVer ? latestTag.raw : latestTag } +async function createReleaseBranch(version: string): Promise { + try { + const versionBranch = `releases/${version}` + const currentBranch = ( + await sh('git', 'rev-parse', '--abbrev-ref', 'HEAD') + ).trim() + if (currentBranch !== versionBranch) { + await sh('git', 'checkout', '-b', versionBranch) + } + } catch (error) { + console.log(`Failed to create release branch: ${error}`) + } +} + /** Converts a string to Channel type if possible */ function parseChannel(arg: string): Channel { if (arg === 'production' || arg === 'beta' || arg === 'test') { @@ -115,6 +129,10 @@ export async function run(args: ReadonlyArray): Promise { }) const nextVersion = getNextVersionNumber(previousVersion, channel) + console.log(`Creating release branch for "${nextVersion}"...`) + createReleaseBranch(nextVersion) + console.log(`Done!`) + console.log(`Setting app version to "${nextVersion}" in app/package.json...`) try { From 41d5ba1209b49b8b43fdf46822af03c4749f9f5c Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 3 Jun 2022 14:07:48 +0200 Subject: [PATCH 61/84] Improve draft-release script to skip release PRs and grab notes from PR body --- script/changelog/api.ts | 2 ++ script/changelog/parser.ts | 44 ++++++++++++++++++++--- script/changelog/test/parser-test.ts | 53 +++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 6 deletions(-) diff --git a/script/changelog/api.ts b/script/changelog/api.ts index 9cd75d7522..f40f5e51f9 100644 --- a/script/changelog/api.ts +++ b/script/changelog/api.ts @@ -3,6 +3,7 @@ import * as HTTPS from 'https' export interface IAPIPR { readonly title: string readonly body: string + readonly headRefName: string } type GraphQLResponse = { @@ -49,6 +50,7 @@ export function fetchPR(id: number): Promise { pullRequest(number: ${id}) { title body + headRefName } } } diff --git a/script/changelog/parser.ts b/script/changelog/parser.ts index 9151c4beab..d7afb1f648 100644 --- a/script/changelog/parser.ts +++ b/script/changelog/parser.ts @@ -37,6 +37,26 @@ function capitalized(str: string): string { return str.charAt(0).toUpperCase() + str.slice(1) } +/** + * Finds a release note in the PR body, which is under the 'Release notes' + * section, preceded by a 'Notes:' title. + * + * @param body Body of the PR to parse + * @returns The release note if it exist, null if it's explicitly marked to + * not have a release note (with no-notes), and undefined if there + * is no 'Release notes' section at all. + */ +export function findReleaseNote(body: string): string | null | undefined { + const re = /^Notes: (.+)$/gm + const matches = re.exec(body) + if (!matches || matches.length < 2) { + return undefined + } + + const note = matches[1].replace(/\.$/, '') + return note === 'no-notes' ? null : note +} + export function findIssueRef(body: string): string { let issueRef = '' @@ -55,7 +75,12 @@ export function findIssueRef(body: string): string { return issueRef } -function getChangelogEntry(commit: IParsedCommit, pr: IAPIPR): string { +function getChangelogEntry(commit: IParsedCommit, pr: IAPIPR): string | null { + let attribution = '' + if (commit.owner !== OfficialOwner) { + attribution = `. Thanks @${commit.owner}!` + } + let type = PlaceholderChangeType const description = capitalized(pr.title) @@ -67,9 +92,12 @@ function getChangelogEntry(commit: IParsedCommit, pr: IAPIPR): string { issueRef = ` #${commit.prID}` } - let attribution = '' - if (commit.owner !== OfficialOwner) { - attribution = `. Thanks @${commit.owner}!` + // Use release note from PR body if defined + const releaseNote = findReleaseNote(pr.body) + if (releaseNote !== undefined) { + return releaseNote === null + ? null + : `${releaseNote} -${issueRef}${attribution}` } return `[${type}] ${description} -${issueRef}${attribution}` @@ -86,9 +114,15 @@ export async function convertToChangelogFormat( if (!pr) { throw new Error(`Unable to get PR from API: ${commit.prID}`) } + // Skip release PRs + if (pr.headRefName.startsWith('releases/')) { + continue + } const entry = getChangelogEntry(commit, pr) - entries.push(entry) + if (entry !== null) { + entries.push(entry) + } } catch (e) { console.warn('Unable to parse line, using the full message.', e) diff --git a/script/changelog/test/parser-test.ts b/script/changelog/test/parser-test.ts index 2d04249a2d..4b771b0c8a 100644 --- a/script/changelog/test/parser-test.ts +++ b/script/changelog/test/parser-test.ts @@ -1,4 +1,4 @@ -import { findIssueRef } from '../parser' +import { findIssueRef, findReleaseNote } from '../parser' describe('changelog/parser', () => { describe('findIssueRef', () => { @@ -54,4 +54,55 @@ quam vel augue.` expect(findIssueRef(body)).toBe(' #2314') }) }) + + describe('findReleaseNote', () => { + it('detected release note at the end of the body', () => { + const body = ` +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer sollicitudin turpis +tempor euismod fermentum. Nullam hendrerit neque eget risus faucibus volutpat. Donec +ultrices, orci quis auctor ultrices, nulla lacus gravida lectus, non rutrum dolor +quam vel augue. + +Notes: [Fixed] Fix lorem impsum dolor sit amet +` + expect(findReleaseNote(body)).toBe( + '[Fixed] Fix lorem impsum dolor sit amet' + ) + }) + + it('removes dot at the end of release note', () => { + const body = ` +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer sollicitudin turpis +tempor euismod fermentum. Nullam hendrerit neque eget risus faucibus volutpat. Donec +ultrices, orci quis auctor ultrices, nulla lacus gravida lectus, non rutrum dolor +quam vel augue. + +Notes: [Fixed] Fix lorem impsum dolor sit amet. +` + expect(findReleaseNote(body)).toBe( + '[Fixed] Fix lorem impsum dolor sit amet' + ) + }) + + it('detected no release notes wanted for the PR', () => { + const body = ` +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer sollicitudin turpis +tempor euismod fermentum. Nullam hendrerit neque eget risus faucibus volutpat. Donec +ultrices, orci quis auctor ultrices, nulla lacus gravida lectus, non rutrum dolor +quam vel augue. + +Notes: no-notes +` + expect(findReleaseNote(body)).toBeNull() + }) + + it('detected no release notes were added to the PR', () => { + const body = ` +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer sollicitudin turpis +tempor euismod fermentum. Nullam hendrerit neque eget risus faucibus volutpat. Donec +ultrices, orci quis auctor ultrices, nulla lacus gravida lectus, non rutrum dolor +quam vel augue.` + expect(findReleaseNote(body)).toBeUndefined() + }) + }) }) From 99cd34a45c5ce0901235f9eeab9f8bc7bd171ade Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Sat, 4 Jun 2022 16:40:19 +0300 Subject: [PATCH 62/84] Add terminate notifications call onClose --- app/src/main-process/main.ts | 6 +++++- app/src/main-process/notifications.ts | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/src/main-process/main.ts b/app/src/main-process/main.ts index e66839461f..13865ccd58 100644 --- a/app/src/main-process/main.ts +++ b/app/src/main-process/main.ts @@ -45,7 +45,10 @@ import { requestNotificationsPermission, showNotification, } from 'desktop-notifications' -import { initializeDesktopNotifications } from './notifications' +import { + initializeDesktopNotifications, + terminateNotificationSystem, +} from './notifications' app.setAppLogsPath() enableSourceMaps() @@ -737,6 +740,7 @@ function createWindow() { window.onClose(() => { mainWindow = null if (!__DARWIN__ && !preventQuit) { + terminateNotificationSystem() app.quit() } }) diff --git a/app/src/main-process/notifications.ts b/app/src/main-process/notifications.ts index 680575cdc4..2645d43dee 100644 --- a/app/src/main-process/notifications.ts +++ b/app/src/main-process/notifications.ts @@ -1,6 +1,7 @@ import { initializeNotifications, onNotificationEvent, + terminateNotifications, } from 'desktop-notifications' import { BrowserWindow } from 'electron' import { findToastActivatorClsid } from '../lib/find-toast-activator-clsid' @@ -35,6 +36,10 @@ export function initializeDesktopNotifications() { initializeNotifications({ toastActivatorClsid: windowsToastActivatorClsid }) } +export function terminateNotificationSystem() { + terminateNotifications() +} + export function installNotificationCallback(window: BrowserWindow) { onNotificationEvent((event, id, userInfo) => { ipcWebContents.send( From e5c9d7751e573c355ca161f31c9a8a06ca50fbd1 Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Mon, 6 Jun 2022 12:39:25 +0300 Subject: [PATCH 63/84] Update terminate function name --- app/src/main-process/main.ts | 6 +----- app/src/main-process/notifications.ts | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/src/main-process/main.ts b/app/src/main-process/main.ts index 13865ccd58..e66839461f 100644 --- a/app/src/main-process/main.ts +++ b/app/src/main-process/main.ts @@ -45,10 +45,7 @@ import { requestNotificationsPermission, showNotification, } from 'desktop-notifications' -import { - initializeDesktopNotifications, - terminateNotificationSystem, -} from './notifications' +import { initializeDesktopNotifications } from './notifications' app.setAppLogsPath() enableSourceMaps() @@ -740,7 +737,6 @@ function createWindow() { window.onClose(() => { mainWindow = null if (!__DARWIN__ && !preventQuit) { - terminateNotificationSystem() app.quit() } }) diff --git a/app/src/main-process/notifications.ts b/app/src/main-process/notifications.ts index 2645d43dee..98717e04d0 100644 --- a/app/src/main-process/notifications.ts +++ b/app/src/main-process/notifications.ts @@ -36,7 +36,7 @@ export function initializeDesktopNotifications() { initializeNotifications({ toastActivatorClsid: windowsToastActivatorClsid }) } -export function terminateNotificationSystem() { +export function terminateDesktopNotifications() { terminateNotifications() } From b857f195bb2bba05520e5a02370cee9e172844f1 Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Mon, 6 Jun 2022 12:40:39 +0300 Subject: [PATCH 64/84] Add call to terminateDesktopNotifications on close --- app/src/main-process/app-window.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/main-process/app-window.ts b/app/src/main-process/app-window.ts index 40c3445945..5d03367530 100644 --- a/app/src/main-process/app-window.ts +++ b/app/src/main-process/app-window.ts @@ -21,7 +21,10 @@ import * as path from 'path' import windowStateKeeper from 'electron-window-state' import * as ipcMain from './ipc-main' import * as ipcWebContents from './ipc-webcontents' -import { installNotificationCallback } from './notifications' +import { + installNotificationCallback, + terminateDesktopNotifications, +} from './notifications' export class AppWindow { private window: Electron.BrowserWindow @@ -107,6 +110,7 @@ export class AppWindow { } nativeTheme.removeAllListeners() autoUpdater.removeAllListeners() + terminateDesktopNotifications() }) if (__WIN32__) { From 262317f36e20b2422723ae6c8578913c36cc449a Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Mon, 6 Jun 2022 16:20:40 +0300 Subject: [PATCH 65/84] Update desktop-notifications version to 0.2.2 --- app/package.json | 2 +- app/yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/package.json b/app/package.json index 6a7777745e..a1819eda2d 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.1", + "desktop-notifications": "^0.2.2", "desktop-trampoline": "desktop/desktop-trampoline#v0.9.8", "dexie": "^3.2.2", "dompurify": "^2.3.3", diff --git a/app/yarn.lock b/app/yarn.lock index 063142356c..7c80ee90e7 100644 --- a/app/yarn.lock +++ b/app/yarn.lock @@ -363,10 +363,10 @@ delegates@^1.0.0: resolved "https://registry.yarnpkg.com/delegates/-/delegates-1.0.0.tgz#84c6e159b81904fdca59a0ef44cd870d31250f9a" integrity sha1-hMbhWbgZBP3KWaDvRM2HDTElD5o= -desktop-notifications@^0.2.1: - version "0.2.1" - resolved "https://registry.yarnpkg.com/desktop-notifications/-/desktop-notifications-0.2.1.tgz#ff934e45ef6b47cfbbd6c596c759fc91b96ab753" - integrity sha512-ssB7SYsWlVgFCN/4KE5QIHM1hH4PXGHmlIYpUa4xR7WDHcjMEU6vrnrAFWx9SnPLVkCNDiWRwSDejlS1VF9BHw== +desktop-notifications@^0.2.2: + version "0.2.2" + resolved "https://registry.yarnpkg.com/desktop-notifications/-/desktop-notifications-0.2.2.tgz#197a32ee504a894ad074793dab285681c533e5ac" + integrity sha512-XMnxdWV6ZfiCzLZNC00n6h30Jt3z8yv21FAPBt/kK6hANI0PaoYWsRKEYya0zMUAX/9lrh429R5OtRGOKWZQSw== dependencies: node-addon-api "^5.0.0" prebuild-install "^7.0.1" From 6e6203893b78156e7debf7a1f131b7e9e61f8339 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Mon, 6 Jun 2022 16:14:27 +0200 Subject: [PATCH 66/84] Bump changelog and version to 3.0.2-beta3 --- app/package.json | 2 +- changelog.json | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/package.json b/app/package.json index a1819eda2d..e353b40bab 100644 --- a/app/package.json +++ b/app/package.json @@ -3,7 +3,7 @@ "productName": "GitHub Desktop", "bundleID": "com.github.GitHubClient", "companyName": "GitHub, Inc.", - "version": "3.0.2-beta2", + "version": "3.0.2-beta3", "main": "./main.js", "repository": { "type": "git", diff --git a/changelog.json b/changelog.json index 142a1b26b6..af4e34cea9 100644 --- a/changelog.json +++ b/changelog.json @@ -1,5 +1,8 @@ { "releases": { + "3.0.2-beta3": [ + "[Fixed] Terminate all GitHub Desktop processes on Windows when the app is closed - #14733. Thanks @tsvetilian-ty!" + ], "3.0.2-beta2": [ "[Fixed] Fix crash launching the app on macOS High Sierra - #14712" ], From e45ffbde8d0560dcf9bbc393db0dac59dbffb665 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Mon, 6 Jun 2022 10:28:41 -0400 Subject: [PATCH 67/84] Better words Co-Authored-By: Sergio Padrino <1083228+sergiou87@users.noreply.github.com> --- app/src/ui/history/selected-commit.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/ui/history/selected-commit.tsx b/app/src/ui/history/selected-commit.tsx index 8b8e7cab13..66fc03fc72 100644 --- a/app/src/ui/history/selected-commit.tsx +++ b/app/src/ui/history/selected-commit.tsx @@ -304,7 +304,7 @@ export class SelectedCommits extends React.Component<

Unable to display diff when multiple{' '} - {enableMultiCommitDiffs() ? 'non-adjacent ' : ' '}commits are + {enableMultiCommitDiffs() ? 'non-consecutive ' : ' '}commits are selected.

You can:
From 7475f5fb83d4dbcaa2063f97058e27eb19c18713 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Mon, 6 Jun 2022 10:37:01 -0400 Subject: [PATCH 68/84] More descriptive about the range Co-authored-by: Sergio Padrino --- app/src/ui/history/selected-commit.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/ui/history/selected-commit.tsx b/app/src/ui/history/selected-commit.tsx index 66fc03fc72..98d6c70ca5 100644 --- a/app/src/ui/history/selected-commit.tsx +++ b/app/src/ui/history/selected-commit.tsx @@ -311,7 +311,7 @@ export class SelectedCommits extends React.Component<
  • Select a single commit{' '} - {enableMultiCommitDiffs() ? 'or a range of commits ' : ' '}to + {enableMultiCommitDiffs() ? 'or a range of consecutive commits ' : ' '}to view a diff.
  • Drag the commits to the branch menu to cherry-pick them.
    • From c6f6b1bdf670fe30eabd7df416ff667b5f76e42d Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Mon, 6 Jun 2022 11:00:02 -0400 Subject: [PATCH 69/84] Lint --- app/src/ui/history/selected-commit.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/src/ui/history/selected-commit.tsx b/app/src/ui/history/selected-commit.tsx index 98d6c70ca5..3b31b63dff 100644 --- a/app/src/ui/history/selected-commit.tsx +++ b/app/src/ui/history/selected-commit.tsx @@ -311,8 +311,10 @@ export class SelectedCommits extends React.Component<
    • Select a single commit{' '} - {enableMultiCommitDiffs() ? 'or a range of consecutive commits ' : ' '}to - view a diff. + {enableMultiCommitDiffs() + ? 'or a range of consecutive commits ' + : ' '} + to view a diff.
    • Drag the commits to the branch menu to cherry-pick them.
    • Drag the commits to squash or reorder them.
      • From 332ef862a3c82c66e0335580e4012c486cf62650 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Mon, 6 Jun 2022 12:51:54 -0400 Subject: [PATCH 70/84] Remember zoom level on update --- app/src/lib/ipc-shared.ts | 1 + app/src/lib/local-storage.ts | 30 +++++++++++++++++++++++++++- app/src/lib/stores/app-store.ts | 32 ++++++++++++++++++++++++++++-- app/src/main-process/app-window.ts | 4 ++++ app/src/main-process/main.ts | 4 ++++ app/src/ui/main-process-proxy.ts | 6 ++++++ 6 files changed, 74 insertions(+), 3 deletions(-) diff --git a/app/src/lib/ipc-shared.ts b/app/src/lib/ipc-shared.ts index 477e69765f..07ffdb7cc0 100644 --- a/app/src/lib/ipc-shared.ts +++ b/app/src/lib/ipc-shared.ts @@ -76,6 +76,7 @@ export type RequestChannels = { 'set-native-theme-source': (themeName: ThemeSource) => void 'focus-window': () => void 'notification-event': NotificationCallback + 'set-window-zoom-factor': (zoomFactor: number) => void } /** diff --git a/app/src/lib/local-storage.ts b/app/src/lib/local-storage.ts index 16f1eb2dc3..efc6c40bba 100644 --- a/app/src/lib/local-storage.ts +++ b/app/src/lib/local-storage.ts @@ -50,7 +50,7 @@ export function setBoolean(key: string, value: boolean) { } /** - * Retrieve a `number` value from a given local storage entry if found, or the + * Retrieve a integer number value from a given local storage entry if found, or the * provided `defaultValue` if the key doesn't exist or if the value cannot be * converted into a number * @@ -77,6 +77,34 @@ export function getNumber( return value } +/** + * Retrieve a floating point number value from a given local storage entry if + * found, or the provided `defaultValue` if the key doesn't exist or if the + * value cannot be converted into a number + * + * @param key local storage entry to read + * @param defaultValue fallback value if unable to find key or valid value + */ +export function getFloatNumber(key: string): number | undefined +export function getFloatNumber(key: string, defaultValue: number): number +export function getFloatNumber( + key: string, + defaultValue?: number +): number | undefined { + const numberAsText = localStorage.getItem(key) + + if (numberAsText === null || numberAsText.length === 0) { + return defaultValue + } + + const value = parseFloat(numberAsText) + if (isNaN(value)) { + return defaultValue + } + + return value +} + /** * Set the provided key in local storage to a numeric value, or update the * existing value if a key is already defined. diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 8a9cfe1323..5ade396f39 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -76,6 +76,7 @@ import { getCurrentWindowZoomFactor, updatePreferredAppMenuItemLabels, updateAccounts, + setWindowZoomFactor, } from '../../ui/main-process-proxy' import { API, @@ -205,6 +206,7 @@ import { getEnum, getObject, setObject, + getFloatNumber, } from '../local-storage' import { ExternalEditorError, suggestedExternalEditor } from '../editors/shared' import { ApiRepositoriesStore } from './api-repositories-store' @@ -575,13 +577,38 @@ export class AppStore extends TypedBaseStore { } private initializeZoomFactor = async () => { - const zoomFactor = await getCurrentWindowZoomFactor() + let zoomFactor = await getCurrentWindowZoomFactor() if (zoomFactor === undefined) { return } + zoomFactor = this.checkZoomFactorLocalStorage(zoomFactor) this.onWindowZoomFactorChanged(zoomFactor) } + /** + * On Windows OS, whenever a user toggles their zoom factor, chromium stores it + * in their `%AppData%/Roaming/GitHub Desktop/Preferences.js` denoted by the + * file path to the application. That file path contains the apps version. + * Thus, on every update, the users set zoom level gets reset as there is not + * defined value for the current app version. + * */ + private checkZoomFactorLocalStorage(zoomFactor: number) { + // One is the default value, we only care about checking the locally stored + // value if it is one because that is the default value after an + // update + if(zoomFactor !== 1 || !__WIN32__) { + return zoomFactor + } + + const locallyStoredZoomFactor = getFloatNumber('zoom-factor') + if(locallyStoredZoomFactor !== undefined && locallyStoredZoomFactor !== zoomFactor) { + setWindowZoomFactor(locallyStoredZoomFactor) + return locallyStoredZoomFactor + } + + return zoomFactor + } + private onTokenInvalidated = (endpoint: string) => { const account = getAccountForEndpoint(this.accounts, endpoint) @@ -802,8 +829,9 @@ export class AppStore extends TypedBaseStore { private onWindowZoomFactorChanged(zoomFactor: number) { const current = this.windowZoomFactor this.windowZoomFactor = zoomFactor - + if (zoomFactor !== current) { + setNumber('zoom-factor', zoomFactor) this.updateResizableConstraints() this.emitUpdate() } diff --git a/app/src/main-process/app-window.ts b/app/src/main-process/app-window.ts index 5d03367530..5e9ed864f7 100644 --- a/app/src/main-process/app-window.ts +++ b/app/src/main-process/app-window.ts @@ -416,6 +416,10 @@ export class AppWindow { return this.window.webContents.zoomFactor } + public setWindowZoomFactor(zoomFactor: number) { + return this.window.webContents.zoomFactor = zoomFactor + } + /** * Method to show the save dialog and return the first file path it returns. */ diff --git a/app/src/main-process/main.ts b/app/src/main-process/main.ts index e66839461f..114480145f 100644 --- a/app/src/main-process/main.ts +++ b/app/src/main-process/main.ts @@ -515,6 +515,10 @@ app.on('ready', () => { mainWindow?.getCurrentWindowZoomFactor() ) + ipcMain.on('set-window-zoom-factor', (_, zoomFactor: number) => + mainWindow?.setWindowZoomFactor(zoomFactor) + ) + /** * An event sent by the renderer asking for a copy of the current * application menu. diff --git a/app/src/ui/main-process-proxy.ts b/app/src/ui/main-process-proxy.ts index 2fa5dee7bb..497e2ad4cb 100644 --- a/app/src/ui/main-process-proxy.ts +++ b/app/src/ui/main-process-proxy.ts @@ -155,6 +155,12 @@ export const getCurrentWindowZoomFactor = invokeProxy( 0 ) +/** Tell the main process to obtain the current window's zoom factor */ +export const setWindowZoomFactor = sendProxy( + 'set-window-zoom-factor', + 1 +) + /** Tell the main process to check for app updates */ export const checkForUpdates = invokeProxy('check-for-updates', 1) From 9b7ddf178e04c6d065a4a0b35f09cb92d7d11737 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Mon, 6 Jun 2022 20:45:14 -0400 Subject: [PATCH 71/84] linter... --- app/src/lib/stores/app-store.ts | 13 ++++++++----- app/src/main-process/app-window.ts | 2 +- app/src/ui/main-process-proxy.ts | 5 +---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 5ade396f39..f90e0d6494 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -585,23 +585,26 @@ export class AppStore extends TypedBaseStore { this.onWindowZoomFactorChanged(zoomFactor) } - /** + /** * On Windows OS, whenever a user toggles their zoom factor, chromium stores it * in their `%AppData%/Roaming/GitHub Desktop/Preferences.js` denoted by the * file path to the application. That file path contains the apps version. * Thus, on every update, the users set zoom level gets reset as there is not * defined value for the current app version. * */ - private checkZoomFactorLocalStorage(zoomFactor: number) { + private checkZoomFactorLocalStorage(zoomFactor: number) { // One is the default value, we only care about checking the locally stored // value if it is one because that is the default value after an // update - if(zoomFactor !== 1 || !__WIN32__) { + if (zoomFactor !== 1 || !__WIN32__) { return zoomFactor } const locallyStoredZoomFactor = getFloatNumber('zoom-factor') - if(locallyStoredZoomFactor !== undefined && locallyStoredZoomFactor !== zoomFactor) { + if ( + locallyStoredZoomFactor !== undefined && + locallyStoredZoomFactor !== zoomFactor + ) { setWindowZoomFactor(locallyStoredZoomFactor) return locallyStoredZoomFactor } @@ -829,7 +832,7 @@ export class AppStore extends TypedBaseStore { private onWindowZoomFactorChanged(zoomFactor: number) { const current = this.windowZoomFactor this.windowZoomFactor = zoomFactor - + if (zoomFactor !== current) { setNumber('zoom-factor', zoomFactor) this.updateResizableConstraints() diff --git a/app/src/main-process/app-window.ts b/app/src/main-process/app-window.ts index 5e9ed864f7..2f38d83216 100644 --- a/app/src/main-process/app-window.ts +++ b/app/src/main-process/app-window.ts @@ -417,7 +417,7 @@ export class AppWindow { } public setWindowZoomFactor(zoomFactor: number) { - return this.window.webContents.zoomFactor = zoomFactor + return (this.window.webContents.zoomFactor = zoomFactor) } /** diff --git a/app/src/ui/main-process-proxy.ts b/app/src/ui/main-process-proxy.ts index 497e2ad4cb..b6b55e894c 100644 --- a/app/src/ui/main-process-proxy.ts +++ b/app/src/ui/main-process-proxy.ts @@ -156,10 +156,7 @@ export const getCurrentWindowZoomFactor = invokeProxy( ) /** Tell the main process to obtain the current window's zoom factor */ -export const setWindowZoomFactor = sendProxy( - 'set-window-zoom-factor', - 1 -) +export const setWindowZoomFactor = sendProxy('set-window-zoom-factor', 1) /** Tell the main process to check for app updates */ export const checkForUpdates = invokeProxy('check-for-updates', 1) From 9df58fd17793c144af731c47a8e15c56fd00bc4b Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Tue, 7 Jun 2022 09:02:42 +0200 Subject: [PATCH 72/84] Wrap repo as a Promise to please Dexie --- app/src/lib/stores/repositories-store.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/lib/stores/repositories-store.ts b/app/src/lib/stores/repositories-store.ts index 3de0de6475..466d70c920 100644 --- a/app/src/lib/stores/repositories-store.ts +++ b/app/src/lib/stores/repositories-store.ts @@ -124,7 +124,7 @@ export class RepositoriesStore extends TypedBaseStore< ) } - return new GitHubRepository( + const ghRepo = new GitHubRepository( repo.name, owner, repo.id, @@ -137,6 +137,10 @@ export class RepositoriesStore extends TypedBaseStore< repo.permissions, parent ) + + // Dexie gets confused if we return a non-promise value (e.g. if this function + // didn't need to await for the parent repo or the owner) + return Promise.resolve(ghRepo) } private async toRepository(repo: IDatabaseRepository) { From 108b4e841067d6a22f6ed1860efc54d5f7af1788 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Tue, 7 Jun 2022 08:26:53 -0400 Subject: [PATCH 73/84] Update app/src/ui/main-process-proxy.ts Co-authored-by: Sergio Padrino --- app/src/ui/main-process-proxy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/ui/main-process-proxy.ts b/app/src/ui/main-process-proxy.ts index b6b55e894c..710c04378c 100644 --- a/app/src/ui/main-process-proxy.ts +++ b/app/src/ui/main-process-proxy.ts @@ -155,7 +155,7 @@ export const getCurrentWindowZoomFactor = invokeProxy( 0 ) -/** Tell the main process to obtain the current window's zoom factor */ +/** Tell the main process to set the current window's zoom factor */ export const setWindowZoomFactor = sendProxy('set-window-zoom-factor', 1) /** Tell the main process to check for app updates */ From fab507ec8ac8fbe1e69c92657a6b85f59b1f8a76 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 7 Jun 2022 08:32:53 -0400 Subject: [PATCH 74/84] Make more succinct --- app/src/lib/stores/app-store.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index f90e0d6494..3374003be2 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -577,11 +577,10 @@ export class AppStore extends TypedBaseStore { } private initializeZoomFactor = async () => { - let zoomFactor = await getCurrentWindowZoomFactor() + const zoomFactor = await this.getWindowZoomFactor() if (zoomFactor === undefined) { return } - zoomFactor = this.checkZoomFactorLocalStorage(zoomFactor) this.onWindowZoomFactorChanged(zoomFactor) } @@ -592,7 +591,8 @@ export class AppStore extends TypedBaseStore { * Thus, on every update, the users set zoom level gets reset as there is not * defined value for the current app version. * */ - private checkZoomFactorLocalStorage(zoomFactor: number) { + private async getWindowZoomFactor() { + const zoomFactor = await getCurrentWindowZoomFactor() // One is the default value, we only care about checking the locally stored // value if it is one because that is the default value after an // update From 43683c3778bd78a5209ca1470ee846ac421bbf50 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 7 Jun 2022 08:37:31 -0400 Subject: [PATCH 75/84] Remove needless return Co-Authored-By: Sergio Padrino <1083228+sergiou87@users.noreply.github.com> --- app/src/main-process/app-window.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main-process/app-window.ts b/app/src/main-process/app-window.ts index 2f38d83216..54fff826f0 100644 --- a/app/src/main-process/app-window.ts +++ b/app/src/main-process/app-window.ts @@ -417,7 +417,7 @@ export class AppWindow { } public setWindowZoomFactor(zoomFactor: number) { - return (this.window.webContents.zoomFactor = zoomFactor) + this.window.webContents.zoomFactor = zoomFactor } /** From eb9f304bff88ea9c1b019d9eda82a41cee7c1019 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Tue, 7 Jun 2022 16:40:44 +0200 Subject: [PATCH 76/84] Bump changelog and version to 3.0.2-beta4 --- app/package.json | 2 +- changelog.json | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/package.json b/app/package.json index e353b40bab..e55baf94cc 100644 --- a/app/package.json +++ b/app/package.json @@ -3,7 +3,7 @@ "productName": "GitHub Desktop", "bundleID": "com.github.GitHubClient", "companyName": "GitHub, Inc.", - "version": "3.0.2-beta3", + "version": "3.0.2-beta4", "main": "./main.js", "repository": { "type": "git", diff --git a/changelog.json b/changelog.json index af4e34cea9..dc370da8fc 100644 --- a/changelog.json +++ b/changelog.json @@ -1,5 +1,10 @@ { "releases": { + "3.0.2-beta4": [ + "[Improved] Add support for SSH password prompts when accessing repositories - #14676", + "[Fixed] Fix Markdown syntax highlighting - #14710", + "[Fixed] Fix issue with some repositories not being properly persisted - #14748" + ], "3.0.2-beta3": [ "[Fixed] Terminate all GitHub Desktop processes on Windows when the app is closed - #14733. Thanks @tsvetilian-ty!" ], From c5ba42f9c25723c733f305815c0c8270ec557a79 Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Tue, 7 Jun 2022 19:16:12 +0300 Subject: [PATCH 77/84] Change separator from space to new line --- app/src/ui/changes/changes-list.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/ui/changes/changes-list.tsx b/app/src/ui/changes/changes-list.tsx index 3873d22ee5..a6ea7f886a 100644 --- a/app/src/ui/changes/changes-list.tsx +++ b/app/src/ui/changes/changes-list.tsx @@ -437,7 +437,7 @@ export class ChangesList extends React.Component< const fullPaths = files.map(file => Path.join(this.props.repository.path, file.path) ) - clipboard.writeText(fullPaths.join(' ')) + clipboard.writeText(fullPaths.join('\r\n')) }, } } @@ -449,7 +449,7 @@ export class ChangesList extends React.Component< label: CopySelectedRelativePathsLabel, action: () => { const paths = files.map(file => Path.normalize(file.path)) - clipboard.writeText(paths.join(' ')) + clipboard.writeText(paths.join('\r\n')) }, } } From 6d53fd1d438f363142fbcef23d77fa3f35da0e6c Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 7 Jun 2022 12:59:21 -0400 Subject: [PATCH 78/84] Collaborator team --- app/src/main-process/squirrel-updater.ts | 2 +- docs/process/teams.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main-process/squirrel-updater.ts b/app/src/main-process/squirrel-updater.ts index 154f97dca8..eb0ef380a4 100644 --- a/app/src/main-process/squirrel-updater.ts +++ b/app/src/main-process/squirrel-updater.ts @@ -10,7 +10,7 @@ const rootAppDir = Path.resolve(appFolder, '..') const updateDotExe = Path.resolve(Path.join(rootAppDir, 'Update.exe')) const exeName = Path.basename(process.execPath) -// A lot of this code was cargo-culted from our Atom comrades: +// A lot of this code was cargo-culted from our Atom collaborators: // https://github.com/atom/atom/blob/7c9f39e3f1d05ee423e0093e6b83f042ce11c90a/src/main-process/squirrel-update.coffee. /** diff --git a/docs/process/teams.md b/docs/process/teams.md index ecaa8e0885..adde6c63ff 100644 --- a/docs/process/teams.md +++ b/docs/process/teams.md @@ -11,7 +11,7 @@ These are good teams to start with for general communication and questions. (Mem | Team | Purpose | |:--|:--| | `@desktop/maintainers` | The people designing, developing, and driving GitHub Desktop. Includes all groups below. | -| `@desktop/comrades` | Community members with a track record of activity in the Desktop project | +| `@desktop/collaborators` | Community members with a track record of activity in the Desktop project | ## Special-purpose Teams From 4f3af0ce50aa7e5d9859904da35898841b360c07 Mon Sep 17 00:00:00 2001 From: Steve Ward Date: Wed, 8 Jun 2022 10:28:13 -0400 Subject: [PATCH 79/84] Create no-response.yml --- .github/workflows/no-response.yml | 32 +++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .github/workflows/no-response.yml diff --git a/.github/workflows/no-response.yml b/.github/workflows/no-response.yml new file mode 100644 index 0000000000..b0ad9909a1 --- /dev/null +++ b/.github/workflows/no-response.yml @@ -0,0 +1,32 @@ + + +name: No Response + +# Both `issue_comment` and `scheduled` event types are required for this Action +# to work properly. +on: + issue_comment: + types: [created] + schedule: + # Schedule for five minutes after the hour, every hour + - cron: '5 * * * *' + +permissions: + issues: write + +jobs: + noResponse: + runs-on: ubuntu-latest + steps: + - uses: lee-dohm/no-response@v0.5.0 + with: + token: ${{ secrets.GITHUB_TOKEN }} + closeComment: > + Thank you for your issue! + + We haven’t gotten a response to our questions above. With only the information + that is currently in the issue, we don’t have enough information to take + action. We’re going to close this but don’t hesitate to reach out if you have + or find the answers we need. If you answer our questions above, a maintainer + will reopen this issue. + daysUntilClose: 7 From e81ecd6d67645a261b2eea3465cc3345f4bc1b17 Mon Sep 17 00:00:00 2001 From: Steve Ward Date: Wed, 8 Jun 2022 10:30:21 -0400 Subject: [PATCH 80/84] Remove probot no-response --- .github/no-response.yml | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 .github/no-response.yml diff --git a/.github/no-response.yml b/.github/no-response.yml deleted file mode 100644 index f182b078b0..0000000000 --- a/.github/no-response.yml +++ /dev/null @@ -1,16 +0,0 @@ -# Configuration for no-response - https://github.com/probot/no-response - -# Number of days of inactivity before an Issue is closed for lack of response -daysUntilClose: 7 -# Label requiring a response -# TODO: also close `needs-reproduction` issues (blocked by https://github.com/probot/no-response/issues/11) -responseRequiredLabel: more-info-needed -# Comment to post when closing an issue due to lack of response. -closeComment: > - Thank you for your issue! - - We haven’t gotten a response to our questions above. With only the information - that is currently in the issue, we don’t have enough information to take - action. We’re going to close this but don’t hesitate to reach out if you have - or find the answers we need. If you answer our questions above, a maintainer - will reopen this issue. From d44085533c0573ef66d227b31e0e1fa56fe09c1b Mon Sep 17 00:00:00 2001 From: Steve Ward Date: Wed, 8 Jun 2022 10:38:30 -0400 Subject: [PATCH 81/84] Add mention that the issue will automatically reopen --- .github/workflows/no-response.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/no-response.yml b/.github/workflows/no-response.yml index b0ad9909a1..62770d99b2 100644 --- a/.github/workflows/no-response.yml +++ b/.github/workflows/no-response.yml @@ -1,5 +1,5 @@ - + name: No Response # Both `issue_comment` and `scheduled` event types are required for this Action @@ -10,7 +10,7 @@ on: schedule: # Schedule for five minutes after the hour, every hour - cron: '5 * * * *' - + permissions: issues: write @@ -23,10 +23,10 @@ jobs: token: ${{ secrets.GITHUB_TOKEN }} closeComment: > Thank you for your issue! - + We haven’t gotten a response to our questions above. With only the information that is currently in the issue, we don’t have enough information to take action. We’re going to close this but don’t hesitate to reach out if you have - or find the answers we need. If you answer our questions above, a maintainer - will reopen this issue. + or find the answers we need. If you answer our questions above, this issue will + automatically reopen. daysUntilClose: 7 From d9d4384a7c7ced1a982faf91a78870a2c137277e Mon Sep 17 00:00:00 2001 From: Steve Ward Date: Wed, 8 Jun 2022 12:33:59 -0400 Subject: [PATCH 82/84] Run linter --- .github/workflows/no-response.yml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/no-response.yml b/.github/workflows/no-response.yml index 62770d99b2..738172862d 100644 --- a/.github/workflows/no-response.yml +++ b/.github/workflows/no-response.yml @@ -1,5 +1,3 @@ - - name: No Response # Both `issue_comment` and `scheduled` event types are required for this Action @@ -24,9 +22,10 @@ jobs: closeComment: > Thank you for your issue! - We haven’t gotten a response to our questions above. With only the information - that is currently in the issue, we don’t have enough information to take - action. We’re going to close this but don’t hesitate to reach out if you have - or find the answers we need. If you answer our questions above, this issue will - automatically reopen. + We haven’t gotten a response to our questions above. With only the + information that is currently in the issue, we don’t have enough + information to take action. We’re going to close this but don’t + hesitate to reach out if you have or find the answers we need. If + you answer our questions above, this issue will automatically + reopen. daysUntilClose: 7 From f220a66d9ec6d6a4e9b9525a6e2e93576667c0fa Mon Sep 17 00:00:00 2001 From: Steve Ward Date: Thu, 9 Jun 2022 10:46:25 -0400 Subject: [PATCH 83/84] Add correct label --- .github/workflows/no-response.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/no-response.yml b/.github/workflows/no-response.yml index 738172862d..44d543dc91 100644 --- a/.github/workflows/no-response.yml +++ b/.github/workflows/no-response.yml @@ -29,3 +29,4 @@ jobs: you answer our questions above, this issue will automatically reopen. daysUntilClose: 7 + responseRequiredLabel: more-info-needed From 21f95dbaffafb58eea783f2a805aa459bd3fc29b Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Fri, 10 Jun 2022 13:43:32 +0300 Subject: [PATCH 84/84] Update to use EOL from os package --- app/src/ui/changes/changes-list.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/ui/changes/changes-list.tsx b/app/src/ui/changes/changes-list.tsx index a6ea7f886a..9970ca7b23 100644 --- a/app/src/ui/changes/changes-list.tsx +++ b/app/src/ui/changes/changes-list.tsx @@ -53,6 +53,7 @@ import { hasConflictedFiles } from '../../lib/status' import { createObservableRef } from '../lib/observable-ref' import { Tooltip, TooltipDirection } from '../lib/tooltip' import { Popup } from '../../models/popup' +import { EOL } from 'os' const RowHeight = 29 const StashIcon: OcticonSymbol.OcticonSymbolType = { @@ -437,7 +438,7 @@ export class ChangesList extends React.Component< const fullPaths = files.map(file => Path.join(this.props.repository.path, file.path) ) - clipboard.writeText(fullPaths.join('\r\n')) + clipboard.writeText(fullPaths.join(EOL)) }, } } @@ -449,7 +450,7 @@ export class ChangesList extends React.Component< label: CopySelectedRelativePathsLabel, action: () => { const paths = files.map(file => Path.normalize(file.path)) - clipboard.writeText(paths.join('\r\n')) + clipboard.writeText(paths.join(EOL)) }, } }