From 64ba21603ce0a13c8482987d523fe3085ca239ee Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Tue, 10 Dec 2024 16:19:08 +0100 Subject: [PATCH 01/17] First take on a simplified CLI, invoking the desktop executable directly macOS only for now --- app/package.json | 1 + app/src/cli/commands/clone.ts | 46 --------- app/src/cli/commands/help.ts | 79 --------------- app/src/cli/commands/open.ts | 39 -------- app/src/cli/dev-commands-global.ts | 4 - app/src/cli/load-commands.ts | 50 ---------- app/src/cli/main.ts | 153 +++++++++++------------------ app/src/cli/open-desktop.ts | 24 ----- app/src/cli/util.ts | 48 --------- app/yarn.lock | 5 + package.json | 1 + yarn.lock | 36 ++----- 12 files changed, 72 insertions(+), 414 deletions(-) delete mode 100644 app/src/cli/commands/clone.ts delete mode 100644 app/src/cli/commands/help.ts delete mode 100644 app/src/cli/commands/open.ts delete mode 100644 app/src/cli/dev-commands-global.ts delete mode 100644 app/src/cli/load-commands.ts delete mode 100644 app/src/cli/open-desktop.ts delete mode 100644 app/src/cli/util.ts diff --git a/app/package.json b/app/package.json index 5e327c9c229..07301d16809 100644 --- a/app/package.json +++ b/app/package.json @@ -44,6 +44,7 @@ "marked": "^4.0.10", "mem": "^4.3.0", "memoize-one": "^4.0.3", + "minimist": "^1.2.8", "mri": "^1.1.0", "p-limit": "^2.2.0", "p-memoize": "^7.1.1", diff --git a/app/src/cli/commands/clone.ts b/app/src/cli/commands/clone.ts deleted file mode 100644 index 012c0531b77..00000000000 --- a/app/src/cli/commands/clone.ts +++ /dev/null @@ -1,46 +0,0 @@ -import * as QueryString from 'querystring' -import { URL } from 'url' - -import { CommandError } from '../util' -import { openDesktop } from '../open-desktop' -import { ICommandModule, mriArgv } from '../load-commands' - -interface ICloneArgs extends mriArgv { - readonly branch?: string -} - -export const command: ICommandModule = { - command: 'clone ', - description: 'Clone a repository', - args: [ - { - name: 'url|slug', - required: true, - description: 'The URL or the GitHub owner/name alias to clone', - type: 'string', - }, - ], - options: { - branch: { - type: 'string', - aliases: ['b'], - description: 'The branch to checkout after cloning', - }, - }, - handler({ _: [cloneUrl], branch }: ICloneArgs) { - if (!cloneUrl) { - throw new CommandError('Clone URL must be specified') - } - try { - const _ = new URL(cloneUrl) - _.toString() // don’t mark as unused - } catch (e) { - // invalid URL, assume a GitHub repo - cloneUrl = `https://github.com/${cloneUrl}` - } - const url = `openRepo/${cloneUrl}?${QueryString.stringify({ - branch, - })}` - openDesktop(url) - }, -} diff --git a/app/src/cli/commands/help.ts b/app/src/cli/commands/help.ts deleted file mode 100644 index 5396000901b..00000000000 --- a/app/src/cli/commands/help.ts +++ /dev/null @@ -1,79 +0,0 @@ -import chalk from 'chalk' - -import { commands, ICommandModule, IOption } from '../load-commands' - -import { dasherizeOption, printTable } from '../util' - -export const command: ICommandModule = { - command: 'help [command]', - description: 'Show the help page for a command', - handler({ _: [command] }) { - if (command) { - printCommandHelp(command, commands[command]) - } else { - printHelp() - } - }, -} - -function printHelp() { - console.log(chalk.underline('Commands:')) - const table: string[][] = [] - for (const commandName of Object.keys(commands)) { - const command = commands[commandName] - table.push([chalk.bold(command.command), command.description]) - } - printTable(table) - console.log( - `\nRun ${chalk.bold( - `github help ${chalk.gray('')}` - )} for details about each command` - ) -} - -function printCommandHelp(name: string, command: ICommandModule) { - if (!command) { - console.log(`Unrecognized command: ${chalk.bold.red.underline(name)}`) - printHelp() - return - } - console.log(`${chalk.gray('github')} ${command.command}`) - if (command.aliases) { - for (const alias of command.aliases) { - console.log(chalk.gray(`github ${alias}`)) - } - } - console.log() - const [title, body] = command.description.split('\n', 1) - console.log(chalk.bold(title)) - if (body) { - console.log(body) - } - const { options, args } = command - if (options) { - console.log(chalk.underline('\nOptions:')) - printTable( - Object.keys(options) - .map(k => [k, options[k]] as [string, IOption]) - .map(([optionName, option]) => [ - [optionName, ...(option.aliases || [])] - .map(dasherizeOption) - .map(x => chalk.bold.blue(x)) - .join(chalk.gray(', ')), - option.description, - chalk.gray(`[${chalk.underline(option.type)}]`), - ]) - ) - } - if (args && args.length) { - console.log(chalk.underline('\nArguments:')) - printTable( - args.map(arg => [ - (arg.required ? chalk.bold : chalk).blue(arg.name), - arg.required ? chalk.gray('(required)') : '', - arg.description, - chalk.gray(`[${chalk.underline(arg.type)}]`), - ]) - ) - } -} diff --git a/app/src/cli/commands/open.ts b/app/src/cli/commands/open.ts deleted file mode 100644 index b5c58d19f68..00000000000 --- a/app/src/cli/commands/open.ts +++ /dev/null @@ -1,39 +0,0 @@ -import chalk from 'chalk' -import * as Path from 'path' - -import { ICommandModule, mriArgv } from '../load-commands' -import { openDesktop } from '../open-desktop' -import { parseRemote } from '../../lib/remote-parsing' - -export const command: ICommandModule = { - command: 'open ', - aliases: [''], - description: 'Open a git repository in GitHub Desktop', - args: [ - { - name: 'path', - description: 'The path to the repository to open', - type: 'string', - required: false, - }, - ], - handler({ _: [pathArg] }: mriArgv) { - if (!pathArg) { - // just open Desktop - openDesktop() - return - } - //Check if the pathArg is a remote url - if (parseRemote(pathArg) != null) { - console.log( - `\nYou cannot open a remote URL in GitHub Desktop\n` + - `Use \`${chalk.bold(`git clone ` + pathArg)}\`` + - ` instead to initiate the clone` - ) - } else { - const repositoryPath = Path.resolve(process.cwd(), pathArg) - const url = `openLocalRepo/${encodeURIComponent(repositoryPath)}` - openDesktop(url) - } - }, -} diff --git a/app/src/cli/dev-commands-global.ts b/app/src/cli/dev-commands-global.ts deleted file mode 100644 index 2199e5e0c45..00000000000 --- a/app/src/cli/dev-commands-global.ts +++ /dev/null @@ -1,4 +0,0 @@ -import { getCLICommands } from '../../../app/app-info' - -const g: any = global -g.__CLI_COMMANDS__ = getCLICommands() diff --git a/app/src/cli/load-commands.ts b/app/src/cli/load-commands.ts deleted file mode 100644 index f988ef4d17e..00000000000 --- a/app/src/cli/load-commands.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { Argv as mriArgv } from 'mri' - -import { TypeName } from './util' - -type StringArray = ReadonlyArray - -export type CommandHandler = (args: mriArgv, argv: StringArray) => void -export { mriArgv } - -export interface IOption { - readonly type: TypeName - readonly aliases?: StringArray - readonly description: string - readonly default?: any -} - -interface IArgument { - readonly name: string - readonly required: boolean - readonly description: string - readonly type: TypeName -} - -export interface ICommandModule { - name?: string - readonly command: string - readonly description: string - readonly handler: CommandHandler - readonly aliases?: StringArray - readonly options?: { [flag: string]: IOption } - readonly args?: ReadonlyArray - readonly unknownOptionHandler?: (flag: string) => void -} - -function loadModule(name: string): ICommandModule { - return require(`./commands/${name}.ts`).command -} - -interface ICommands { - [command: string]: ICommandModule -} -export const commands: ICommands = {} - -for (const fileName of __CLI_COMMANDS__) { - const mod = loadModule(fileName) - if (!mod.name) { - mod.name = fileName - } - commands[mod.name] = mod -} diff --git a/app/src/cli/main.ts b/app/src/cli/main.ts index c9525da9b0c..e8b6fc3b1eb 100644 --- a/app/src/cli/main.ts +++ b/app/src/cli/main.ts @@ -1,107 +1,68 @@ -import mri, { - DictionaryObject, - Options as MriOptions, - ArrayOrString, -} from 'mri' -import chalk from 'chalk' +import { join, resolve } from 'path' +import parse from 'minimist' +import { spawn } from 'child_process' -import { dasherizeOption, CommandError } from './util' -import { commands } from './load-commands' -const defaultCommand = 'open' - -let args = process.argv.slice(2) -if (!args[0]) { - args[0] = '.' +const _spawn = (args: string[]) => { + if (__DARWIN__) { + const execPath = join(__dirname, '../../../').replace(/\/$/, '') + return spawn('open', [execPath, '--args', ...args], { + stdio: ['ignore', 'inherit', 'inherit'], + }) + } else if (__WIN32__) { + throw new Error('TODO') + } else { + throw new Error('Unsupported platform') + } } -const commandArg = args[0] -args = args.slice(1) -const supportsCommand = (name: string) => Object.hasOwn(commands, name) +const run = (...args: Array) => + _spawn(args.filter(x => typeof x === 'string')) + .on('error', e => { + console.error('Error running command', e) + process.exitCode = 1 + }) + .on('exit', code => { + process.exitCode = typeof code === 'number' ? code : process.exitCode + }) -;(function attemptRun(name: string) { - try { - if (supportsCommand(name)) { - runCommand(name) - } else if (name.startsWith('--')) { - attemptRun(name.slice(2)) - } else { - try { - args.unshift(commandArg) - runCommand(defaultCommand) - } catch (err) { - logError(err) - args = [] - runCommand('help') - } - } - } catch (err) { - logError(err) - args = [name] - runCommand('help') - } -})(commandArg) +const args = parse(process.argv.slice(2), { + alias: { help: 'h', branch: 'b' }, + boolean: ['help'], + string: ['branch'], +}) -function logError(err: CommandError) { - console.log(chalk.bgBlack.red('ERR!'), err.message) - if (err.stack && !err.pretty) { - console.log(chalk.gray(err.stack)) - } +const usage = (exitCode = 1): never => { + process.stderr.write( + 'GitHub Desktop CLI usage: \n' + + ' github Open the current directory\n' + + ' github open [path] Open the provided path\n' + + ' github clone [-b branch] Clone the repository by url or \n' + + ' name/owner (ex torvalds/linux),\n' + + ' optionally checking out the branch\n' + ) + process.exit(exitCode) } -console.log() // nice blank line before the command prompt +delete process.env.ELECTRON_RUN_AS_NODE -interface IMRIOpts extends MriOptions { - alias: DictionaryObject - boolean: Array - default: DictionaryObject - string: Array -} - -function runCommand(name: string) { - const command = commands[name] - const opts: IMRIOpts = { - alias: {}, - boolean: [], - default: {}, - string: [], - } - if (command.options) { - for (const flag of Object.keys(command.options)) { - const flagOptions = command.options[flag] - if (flagOptions.aliases) { - opts.alias[flag] = flagOptions.aliases - } - if (Object.hasOwn(flagOptions, 'default')) { - opts.default[flag] = flagOptions.default - } - switch (flagOptions.type) { - case 'string': - opts.string.push(flag) - break - case 'boolean': - opts.boolean.push(flag) - break - } - } - opts.unknown = command.unknownOptionHandler - } - const parsedArgs = mri(args, opts) - if (command.options) { - for (const flag of Object.keys(parsedArgs)) { - if (!(flag in command.options)) { - continue - } +if (args.help || args._.at(0) === 'help') { + usage(0) +} else if (args._.at(0) === 'clone') { + const urlArg = args._.at(1) + // Assume name with owner slug if it looks like it + const url = + urlArg && /^[^\/]+\/[^\/]+$/.test(urlArg) + ? `https://github.com/${urlArg}` + : urlArg - const value = parsedArgs[flag] - const expectedType = command.options[flag].type - if (typeof value !== expectedType) { - throw new CommandError( - `Value passed to flag ${dasherizeOption( - flag - )} was of type ${typeof value}, but was expected to be of type ${expectedType}` - ) - } - } + if (!url) { + usage(1) + } else { + run(`--cli-clone=${url}`, args.branch && `--cli-branch=${args.branch}`) } - command.handler(parsedArgs, args) +} else { + const [firstArg, secondArg] = args._ + const pathArg = firstArg === 'open' ? secondArg : firstArg + const path = resolve(pathArg ?? '.') + run(`--cli-open=${path}`) } diff --git a/app/src/cli/open-desktop.ts b/app/src/cli/open-desktop.ts deleted file mode 100644 index 49091d188c0..00000000000 --- a/app/src/cli/open-desktop.ts +++ /dev/null @@ -1,24 +0,0 @@ -import * as ChildProcess from 'child_process' - -export function openDesktop(url: string = '') { - const env = { ...process.env } - // NB: We're gonna launch Desktop and we definitely don't want to carry over - // `ELECTRON_RUN_AS_NODE`. This seems to only happen on Windows. - delete env['ELECTRON_RUN_AS_NODE'] - - url = 'x-github-client://' + url - - if (__DARWIN__) { - return ChildProcess.spawn('open', [url], { env }) - } else if (__WIN32__) { - // https://github.com/nodejs/node/blob/b39dabefe6d/lib/child_process.js#L565-L577 - const shell = process.env.comspec || 'cmd.exe' - return ChildProcess.spawn(shell, ['/d', '/c', 'start', url], { env }) - } else if (__LINUX__) { - return ChildProcess.spawn('xdg-open', [url], { env }) - } else { - throw new Error( - `Desktop command line interface not currently supported on platform ${process.platform}` - ) - } -} diff --git a/app/src/cli/util.ts b/app/src/cli/util.ts deleted file mode 100644 index 5bfabe4289d..00000000000 --- a/app/src/cli/util.ts +++ /dev/null @@ -1,48 +0,0 @@ -import stripAnsi from 'strip-ansi' - -export type TypeName = - | 'string' - | 'number' - | 'boolean' - | 'symbol' - | 'undefined' - | 'object' - | 'function' - -export class CommandError extends Error { - public pretty = true -} - -export const dasherizeOption = (option: string) => { - if (option.length === 1) { - return '-' + option - } else { - return '--' + option - } -} - -export function printTable(table: string[][]) { - const columnWidths = calculateColumnWidths(table) - for (const row of table) { - let rowStr = ' ' - row.forEach((item, i) => { - rowStr += item - const neededSpaces = columnWidths[i] - stripAnsi(item).length - rowStr += ' '.repeat(neededSpaces + 2) - }) - console.log(rowStr) - } -} - -function calculateColumnWidths(table: string[][]) { - const columnWidths: number[] = Array(table[0].length).fill(0) - for (const row of table) { - row.forEach((item, i) => { - const width = stripAnsi(item).length - if (columnWidths[i] < width) { - columnWidths[i] = width - } - }) - } - return columnWidths -} diff --git a/app/yarn.lock b/app/yarn.lock index e47d321045b..006e5e88b40 100644 --- a/app/yarn.lock +++ b/app/yarn.lock @@ -765,6 +765,11 @@ minimist@^1.2.0, minimist@^1.2.3, minimist@^1.2.5: resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.6.tgz#8637a5b759ea0d6e98702cfb3a9283323c93af44" integrity sha512-Jsjnk4bw3YJqYzbdyBiNsPWHPfO++UGG749Cxs6peCu5Xg4nrena6OVxOYxrQTqww0Jmwt+Ref8rggumkTLz9Q== +minimist@^1.2.8: + version "1.2.8" + resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.8.tgz#c1a464e7693302e082a075cee0c057741ac4772c" + integrity sha512-2yyAR8qBkN3YuheJanUpWC5U3bb5osDywNB8RzDVlDwDHbocAJveqqj1u8+SVD7jkWT4yvsHCpWqqWqAxb0zCA== + mkdirp-classic@^0.5.3: version "0.5.3" resolved "https://registry.yarnpkg.com/mkdirp-classic/-/mkdirp-classic-0.5.3.tgz#fa10c9115cc6d8865be221ba47ee9bed78601113" diff --git a/package.json b/package.json index ab8bd621697..c1536f2faa7 100644 --- a/package.json +++ b/package.json @@ -122,6 +122,7 @@ "@types/lodash": "^4.14.178", "@types/marked": "^4.0.1", "@types/memoize-one": "^3.1.1", + "@types/minimist": "^1.2.5", "@types/mri": "^1.1.0", "@types/node": "22.7.4", "@types/parse-dds": "^1.0.3", diff --git a/yarn.lock b/yarn.lock index 4d7f25adf06..44ab67e3a4b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1338,6 +1338,11 @@ resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.3.tgz#3dca0e3f33b200fc7d1139c0cd96c1268cadfd9d" integrity sha512-tHq6qdbT9U1IRSGf14CL0pUlULksvY9OZ+5eEgl1N7t+OA3tGvNpxJCzuKQlsNgCVwbAs670L1vcVQi8j9HjnA== +"@types/minimist@^1.2.5": + version "1.2.5" + resolved "https://registry.yarnpkg.com/@types/minimist/-/minimist-1.2.5.tgz#ec10755e871497bcd83efe927e43ec46e8c0747e" + integrity sha512-hov8bUuiLiyFPGyFPE1lwWhmzYbirOXQNNo40+y3zow8aFVTeyn3VWL0VFFfdNddA8S4Vf0Tc062rzyNr7Paag== + "@types/mri@^1.1.0": version "1.1.0" resolved "https://registry.yarnpkg.com/@types/mri/-/mri-1.1.0.tgz#66555e4d797713789ea0fefdae0898d8170bf5af" @@ -7557,7 +7562,7 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0": +"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -7575,15 +7580,6 @@ string-width@^4.1.0, string-width@^4.2.0: is-fullwidth-code-point "^3.0.0" strip-ansi "^6.0.0" -string-width@^4.2.3: - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - string-width@^5.0.1, string-width@^5.1.2: version "5.1.2" resolved "https://registry.yarnpkg.com/string-width/-/string-width-5.1.2.tgz#14f8daec6d81e7221d2a357e668cab73bdbca794" @@ -7698,7 +7694,7 @@ string.prototype.trimstart@^1.0.7: define-properties "^1.2.0" es-abstract "^1.22.1" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -7712,13 +7708,6 @@ strip-ansi@^3.0.0: dependencies: ansi-regex "^2.0.0" -strip-ansi@^6.0.0, strip-ansi@^6.0.1: - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - strip-ansi@^7.0.1: version "7.1.0" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45" @@ -8501,16 +8490,7 @@ worker-farm@^1.3.1: errno "^0.1.4" xtend "^4.0.1" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - -wrap-ansi@^7.0.0: +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== From 4193a0c54e49f6b68299245a1d666d0791850487 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 10:51:10 +0100 Subject: [PATCH 02/17] Handle CLI actions separately from url actions --- app/src/lib/cli-action.ts | 10 +++++ app/src/lib/ipc-shared.ts | 1 + app/src/lib/parse-app-url.ts | 15 ------- app/src/main-process/app-window.ts | 8 ++++ app/src/main-process/main.ts | 51 ++++++++++++++++++----- app/src/ui/dispatcher/dispatcher.ts | 64 ++++++++++++++++------------- app/src/ui/index.tsx | 4 ++ 7 files changed, 100 insertions(+), 53 deletions(-) create mode 100644 app/src/lib/cli-action.ts diff --git a/app/src/lib/cli-action.ts b/app/src/lib/cli-action.ts new file mode 100644 index 00000000000..2e0072b421c --- /dev/null +++ b/app/src/lib/cli-action.ts @@ -0,0 +1,10 @@ +export type CLIAction = + | { + readonly kind: 'open-repository' + readonly path: string + } + | { + readonly kind: 'clone-url' + readonly url: string + readonly branch?: string + } diff --git a/app/src/lib/ipc-shared.ts b/app/src/lib/ipc-shared.ts index 8b6387b37e8..750b8c786f5 100644 --- a/app/src/lib/ipc-shared.ts +++ b/app/src/lib/ipc-shared.ts @@ -57,6 +57,7 @@ export type RequestChannels = { 'app-menu': (menu: IMenu) => void 'launch-timing-stats': (stats: ILaunchStats) => void 'url-action': (action: URLActionType) => void + 'cli-action': (action: CLIAction) => void 'certificate-error': ( certificate: Electron.Certificate, error: string, diff --git a/app/src/lib/parse-app-url.ts b/app/src/lib/parse-app-url.ts index 50f115e8c2f..a81d2f418ef 100644 --- a/app/src/lib/parse-app-url.ts +++ b/app/src/lib/parse-app-url.ts @@ -23,13 +23,6 @@ export interface IOpenRepositoryFromURLAction { readonly filepath: string | null } -export interface IOpenRepositoryFromPathAction { - readonly name: 'open-repository-from-path' - - /** The local path to open. */ - readonly path: string -} - export interface IUnknownAction { readonly name: 'unknown' readonly url: string @@ -38,7 +31,6 @@ export interface IUnknownAction { export type URLActionType = | IOAuthAction | IOpenRepositoryFromURLAction - | IOpenRepositoryFromPathAction | IUnknownAction // eslint-disable-next-line @typescript-eslint/naming-convention @@ -132,12 +124,5 @@ export function parseAppURL(url: string): URLActionType { } } - if (actionName === 'openlocalrepo') { - return { - name: 'open-repository-from-path', - path: decodeURIComponent(parsedPath), - } - } - return unknown } diff --git a/app/src/main-process/app-window.ts b/app/src/main-process/app-window.ts index c5ee06f0560..0a2435f7f9e 100644 --- a/app/src/main-process/app-window.ts +++ b/app/src/main-process/app-window.ts @@ -28,6 +28,7 @@ import { } from './notifications' import { addTrustedIPCSender } from './trusted-ipc-sender' import { getUpdaterGUID } from '../lib/get-updater-guid' +import { CLIAction } from '../lib/cli-action' export class AppWindow { private window: Electron.BrowserWindow @@ -312,6 +313,13 @@ export class AppWindow { ipcWebContents.send(this.window.webContents, 'url-action', action) } + /** Send the URL action to the renderer. */ + public sendCLIAction(action: CLIAction) { + this.show() + + ipcWebContents.send(this.window.webContents, 'cli-action', action) + } + /** Send the app launch timing stats to the renderer. */ public sendLaunchTimingStats(stats: ILaunchStats) { ipcWebContents.send(this.window.webContents, 'launch-timing-stats', stats) diff --git a/app/src/main-process/main.ts b/app/src/main-process/main.ts index 070663f9961..e01bebc124b 100644 --- a/app/src/main-process/main.ts +++ b/app/src/main-process/main.ts @@ -50,6 +50,8 @@ import { showNotification, } from 'desktop-notifications' import { initializeDesktopNotifications } from './notifications' +import parseCommandLineArgs from 'minimist' +import { CLIAction } from '../lib/cli-action' app.setAppLogsPath() enableSourceMaps() @@ -139,22 +141,20 @@ process.on('uncaughtException', (error: Error) => { let handlingSquirrelEvent = false if (__WIN32__ && process.argv.length > 1) { const arg = process.argv[1] - const promise = handleSquirrelEvent(arg) + if (promise) { handlingSquirrelEvent = true promise - .catch(e => { - log.error(`Failed handling Squirrel event: ${arg}`, e) - }) - .then(() => { - app.quit() - }) - } else { - handlePossibleProtocolLauncherArgs(process.argv) + .catch(e => log.error(`Failed handling Squirrel event: ${arg}`, e)) + .then(() => app.quit()) } } +if (!handlingSquirrelEvent) { + handleCommandLineArguments(process.argv) +} + initializeDesktopNotifications() function handleAppURL(url: string) { @@ -190,7 +190,7 @@ if (!handlingSquirrelEvent) { mainWindow.focus() } - handlePossibleProtocolLauncherArgs(args) + handleCommandLineArguments(args) }) if (isDuplicateInstance) { @@ -236,6 +236,37 @@ if (__DARWIN__) { }) } +async function handleCommandLineArguments(argv: string[]) { + const args = parseCommandLineArgs(argv) + + if (__WIN32__ && typeof args['protocol-launcher'] === 'string') { + handleAppURL(args['protocol-launcher']) + return + } + + if (typeof args['cli-open'] === 'string') { + handleCLIAction({ kind: 'open-repository', path: args['cli-open'] }) + } else if (typeof args['cli-clone'] === 'string') { + handleCLIAction({ + kind: 'clone-url', + url: args['cli-clone'], + branch: + typeof args['cli-branch'] === 'string' ? args['cli-branch'] : undefined, + }) + } + + return +} + +function handleCLIAction(action: CLIAction) { + onDidLoad(window => { + // This manual focus call _shouldn't_ be necessary, but is for Chrome on + // macOS. See https://github.com/desktop/desktop/issues/973. + window.focus() + window.sendCLIAction(action) + }) +} + /** * Attempt to detect and handle any protocol handler arguments passed * either via the command line directly to the current process or through diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index 06d19509152..622cfce8967 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -33,6 +33,7 @@ import { getCommitsBetweenCommits, getBranches, getRebaseSnapshot, + getRepositoryType, } from '../../lib/git' import { isGitOnPath } from '../../lib/is-git-on-path' import { @@ -121,7 +122,8 @@ import { UnreachableCommitsTab } from '../history/unreachable-commits-dialog' import { sendNonFatalException } from '../../lib/helpers/non-fatal-exception' import { SignInResult } from '../../lib/stores/sign-in-store' import { ICustomIntegration } from '../../lib/custom-integration' -import { dirname, isAbsolute } from 'path' +import { isAbsolute } from 'path' +import { CLIAction } from '../../lib/cli-action' /** * An error handler function. @@ -1860,6 +1862,39 @@ export class Dispatcher { return repository } + public async dispatchCLIAction(action: CLIAction) { + if (action.kind === 'clone-url') { + const { branch, url } = action + + if (branch) { + await this.openBranchNameFromUrl(url, branch) + } else { + await this.openOrCloneRepository(url) + } + } else if (action.kind === 'open-repository') { + // user may accidentally provide a folder within the repository + // this ensures we use the repository root, if it is actually a repository + // otherwise we consider it an untracked repository + const path = await getRepositoryType(action.path) + .then(t => + t.kind === 'regular' ? t.topLevelWorkingDirectory : action.path + ) + .catch(e => { + log.error('Could not determine repository type', e) + return action.path + }) + + const { repositories } = this.appStore.getState() + const existingRepository = matchExistingRepository(repositories, path) + + if (existingRepository) { + await this.selectRepository(existingRepository) + } else { + await this.showPopup({ type: PopupType.AddRepository, path }) + } + } + } + public async dispatchURLAction(action: URLActionType): Promise { switch (action.name) { case 'oauth': @@ -1882,33 +1917,6 @@ export class Dispatcher { this.openRepositoryFromUrl(action) break - case 'open-repository-from-path': - // user may accidentally provide a folder within the repository - // this ensures we use the repository root, if it is actually a repository - // otherwise we consider it an untracked repository - const { repositories } = this.appStore.getState() - - let repo - let path = action.path - - while (!(repo = matchExistingRepository(repositories, path))) { - const parent = dirname(path) - if (parent === path) { - break - } - path = parent - } - - if (repo) { - await this.selectRepository(repo) - } else { - await this.showPopup({ - type: PopupType.AddRepository, - path: action.path, - }) - } - break - default: const unknownAction: IUnknownAction = action log.warn( diff --git a/app/src/ui/index.tsx b/app/src/ui/index.tsx index 6ab8603a545..a74a57ad3b3 100644 --- a/app/src/ui/index.tsx +++ b/app/src/ui/index.tsx @@ -352,6 +352,10 @@ ipcRenderer.on('url-action', (_, action) => dispatcher.dispatchURLAction(action) ) +ipcRenderer.on('cli-action', (_, action) => { + dispatcher.dispatchCLIAction(action) +}) + // react-virtualized will use the literal string "grid" as the 'aria-label' // attribute unless we override it. This is a problem because aria-label should // not be set unless there's a compelling reason for it[1]. From effb8cd1b75e4f9addc64357c57944a63a2062a7 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 11:34:08 +0100 Subject: [PATCH 03/17] Get rid of CLI dynamic commands lookup --- app/app-info.ts | 16 ---------------- docs/technical/placeholders.md | 1 - package.json | 2 +- 3 files changed, 1 insertion(+), 18 deletions(-) diff --git a/app/app-info.ts b/app/app-info.ts index 8c8fd1acbb3..b1317a989da 100644 --- a/app/app-info.ts +++ b/app/app-info.ts @@ -1,27 +1,12 @@ -import * as fs from 'fs' -import * as Path from 'path' - import { getSHA } from './git-info' import { getUpdatesURL, getChannel } from '../script/dist-info' import { version, productName } from './package.json' -const projectRoot = Path.dirname(__dirname) - const devClientId = '3a723b10ac5575cc5bb9' const devClientSecret = '22c34d87789a365981ed921352a7b9a8c3f69d54' const channel = getChannel() -export function getCLICommands() { - return ( - // eslint-disable-next-line no-sync - fs - .readdirSync(Path.resolve(projectRoot, 'app', 'src', 'cli', 'commands')) - .filter(name => name.endsWith('.ts')) - .map(name => name.replace(/\.ts$/, '')) - ) -} - const s = JSON.stringify export function getReplacements() { @@ -41,7 +26,6 @@ export function getReplacements() { __RELEASE_CHANNEL__: s(channel), __UPDATES_URL__: s(getUpdatesURL()), __SHA__: s(getSHA()), - __CLI_COMMANDS__: s(getCLICommands()), 'process.platform': s(process.platform), 'process.env.NODE_ENV': s(process.env.NODE_ENV || 'development'), 'process.env.TEST_ENV': s(process.env.TEST_ENV), diff --git a/docs/technical/placeholders.md b/docs/technical/placeholders.md index 2488269da2b..8cecb5f8f67 100644 --- a/docs/technical/placeholders.md +++ b/docs/technical/placeholders.md @@ -46,7 +46,6 @@ function getReplacements() { __RELEASE_CHANNEL__: s(channel), __UPDATES_URL__: s(distInfo.getUpdatesURL()), __SHA__: s(gitInfo.getSHA()), - __CLI_COMMANDS__: s(getCLICommands()), 'process.platform': s(process.platform), 'process.env.NODE_ENV': s(process.env.NODE_ENV || 'development'), 'process.env.TEST_ENV': s(process.env.TEST_ENV), diff --git a/package.json b/package.json index c1536f2faa7..d59d46799e7 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ }, "description": "GitHub Desktop build dependencies", "scripts": { - "cli": "ts-node --require ./app/test/globals.ts --require ./app/src/cli/dev-commands-global.ts app/src/cli/main.ts", + "cli": "ts-node app/src/cli/main.ts", "check:eslint": "tsc -P eslint-rules/", "test:eslint": "jest eslint-rules/tests/*.test.js", "test:unit": "cross-env ELECTRON_RUN_AS_NODE=1 NODE_OPTIONS='--max_old_space_size=4096' ./node_modules/.bin/electron ./node_modules/jest/bin/jest --detectOpenHandles --silent --testLocationInResults --config ./app/jest.unit.config.js", From 3baa284e7878b5563a1d5eb41a8f7eff83eeefbb Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 11:34:29 +0100 Subject: [PATCH 04/17] Use process.platform over compile time vars --- app/src/cli/main.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/cli/main.ts b/app/src/cli/main.ts index e8b6fc3b1eb..66b0199f5c2 100644 --- a/app/src/cli/main.ts +++ b/app/src/cli/main.ts @@ -3,12 +3,12 @@ import parse from 'minimist' import { spawn } from 'child_process' const _spawn = (args: string[]) => { - if (__DARWIN__) { + if (process.platform === 'darwin') { const execPath = join(__dirname, '../../../').replace(/\/$/, '') return spawn('open', [execPath, '--args', ...args], { stdio: ['ignore', 'inherit', 'inherit'], }) - } else if (__WIN32__) { + } else if (process.platform === 'win32') { throw new Error('TODO') } else { throw new Error('Unsupported platform') From 9cbcc1b1440b950f18edbd4f07a9f11e733c7247 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 11:35:52 +0100 Subject: [PATCH 05/17] Get rid of handlePossibleProtocolLauncherArgs --- app/src/lib/ipc-shared.ts | 1 + app/src/main-process/main.ts | 61 +++++++---------------------- app/test/unit/parse-app-url-test.ts | 24 ------------ 3 files changed, 15 insertions(+), 71 deletions(-) diff --git a/app/src/lib/ipc-shared.ts b/app/src/lib/ipc-shared.ts index 750b8c786f5..f6539023518 100644 --- a/app/src/lib/ipc-shared.ts +++ b/app/src/lib/ipc-shared.ts @@ -16,6 +16,7 @@ import { ThemeSource } from '../ui/lib/theme-source' import { DesktopNotificationPermission } from 'desktop-notifications/dist/notification-permission' import { NotificationCallback } from 'desktop-notifications/dist/notification-callback' import { DesktopAliveEvent } from './stores/alive-store' +import { CLIAction } from './cli-action' /** * Defines the simplex IPC channel names we use from the renderer diff --git a/app/src/main-process/main.ts b/app/src/main-process/main.ts index e01bebc124b..3b1a886f150 100644 --- a/app/src/main-process/main.ts +++ b/app/src/main-process/main.ts @@ -10,7 +10,6 @@ import { nativeTheme, } from 'electron' import * as Fs from 'fs' -import * as URL from 'url' import { AppWindow } from './app-window' import { buildDefaultMenu, getAllMenuItems } from './menu' @@ -52,6 +51,7 @@ import { import { initializeDesktopNotifications } from './notifications' import parseCommandLineArgs from 'minimist' import { CLIAction } from '../lib/cli-action' +import { tryParseUrl } from '../lib/try-parse-url' app.setAppLogsPath() enableSourceMaps() @@ -229,9 +229,9 @@ if (__DARWIN__) { return } - handleAppURL( - `x-github-client://openLocalRepo/${encodeURIComponent(path)}` - ) + // Yeah this isn't technically a CLI action we use it here to indicate + // that it's more trusted than a URL action. + handleCLIAction({ kind: 'open-repository', path }) }) }) } @@ -239,8 +239,17 @@ if (__DARWIN__) { async function handleCommandLineArguments(argv: string[]) { const args = parseCommandLineArgs(argv) + // Desktop registers it's protocol handler callback on Windows as + // `[executable path] --protocol-launcher "%1"`. Note that extra command + // line arguments might be added by Chromium + // (https://electronjs.org/docs/api/app#event-second-instance). if (__WIN32__ && typeof args['protocol-launcher'] === 'string') { - handleAppURL(args['protocol-launcher']) + const url = tryParseUrl(args['protocol-launcher']) + if (url) { + handleAppURL(url.href) + } else { + log.error(`Malformed protocol launcher URL: ${args['protocol-launcher']}`) + } return } @@ -267,48 +276,6 @@ function handleCLIAction(action: CLIAction) { }) } -/** - * Attempt to detect and handle any protocol handler arguments passed - * either via the command line directly to the current process or through - * IPC from a duplicate instance (see makeSingleInstance) - * - * @param args Essentially process.argv, i.e. the first element is the exec - * path - */ -function handlePossibleProtocolLauncherArgs(args: ReadonlyArray) { - log.info(`Received possible protocol arguments: ${args.length}`) - - if (__WIN32__) { - // Desktop registers it's protocol handler callback on Windows as - // `[executable path] --protocol-launcher "%1"`. Note that extra command - // line arguments might be added by Chromium - // (https://electronjs.org/docs/api/app#event-second-instance). - // At launch Desktop checks for that exact scenario here before doing any - // processing. If there's more than one matching url argument because of a - // malformed or untrusted url then we bail out. - - const matchingUrls = args.filter(arg => { - // sometimes `URL.parse` throws an error - try { - const url = URL.parse(arg) - // i think this `slice` is just removing a trailing `:` - return url.protocol && possibleProtocols.has(url.protocol.slice(0, -1)) - } catch (e) { - log.error(`Unable to parse argument as URL: ${arg}`) - return false - } - }) - - if (args.includes(protocolLauncherArg) && matchingUrls.length === 1) { - handleAppURL(matchingUrls[0]) - } else { - log.error(`Malformed launch arguments received: ${args}`) - } - } else if (args.length > 1) { - handleAppURL(args[1]) - } -} - /** * Wrapper around app.setAsDefaultProtocolClient that adds our * custom prefix command line switches on Windows. diff --git a/app/test/unit/parse-app-url-test.ts b/app/test/unit/parse-app-url-test.ts index 058f2020fa4..b297658002f 100644 --- a/app/test/unit/parse-app-url-test.ts +++ b/app/test/unit/parse-app-url-test.ts @@ -1,7 +1,6 @@ import { parseAppURL, IOpenRepositoryFromURLAction, - IOpenRepositoryFromPathAction, IOAuthAction, } from '../../src/lib/parse-app-url' @@ -155,27 +154,4 @@ describe('parseAppURL', () => { expect(openRepo.filepath).toBe('Octokit.Reactive/Octokit.Reactive.csproj') }) }) - - describe('openLocalRepo', () => { - it('parses local paths', () => { - const path = __WIN32__ - ? 'C:\\Users\\johnsmith\\repo' - : '/Users/johnsmith/repo' - const result = parseAppURL( - `x-github-client://openLocalRepo/${encodeURIComponent(path)}` - ) - expect(result.name).toBe('open-repository-from-path') - - const openRepo = result as IOpenRepositoryFromPathAction - expect(openRepo.path).toBe(path) - }) - - it('deals with not having a local path', () => { - let result = parseAppURL(`x-github-client://openLocalRepo/`) - expect(result.name).toBe('unknown') - - result = parseAppURL(`x-github-client://openLocalRepo`) - expect(result.name).toBe('unknown') - }) - }) }) From 9bc426dd9235a62051f2cf09d7a119e6e44a0466 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 11:36:59 +0100 Subject: [PATCH 06/17] Let handleAppUrl take care of parsing problems --- app/src/main-process/main.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/src/main-process/main.ts b/app/src/main-process/main.ts index 3b1a886f150..67baa1e3ae3 100644 --- a/app/src/main-process/main.ts +++ b/app/src/main-process/main.ts @@ -244,12 +244,7 @@ async function handleCommandLineArguments(argv: string[]) { // line arguments might be added by Chromium // (https://electronjs.org/docs/api/app#event-second-instance). if (__WIN32__ && typeof args['protocol-launcher'] === 'string') { - const url = tryParseUrl(args['protocol-launcher']) - if (url) { - handleAppURL(url.href) - } else { - log.error(`Malformed protocol launcher URL: ${args['protocol-launcher']}`) - } + handleAppURL(args['protocol-launcher']) return } From 0201be7705fd83e1fe0d59e70a36f3ec3348805f Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 11:37:26 +0100 Subject: [PATCH 07/17] Remove import --- app/src/main-process/main.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main-process/main.ts b/app/src/main-process/main.ts index 67baa1e3ae3..81cc75ffd68 100644 --- a/app/src/main-process/main.ts +++ b/app/src/main-process/main.ts @@ -51,7 +51,6 @@ import { import { initializeDesktopNotifications } from './notifications' import parseCommandLineArgs from 'minimist' import { CLIAction } from '../lib/cli-action' -import { tryParseUrl } from '../lib/try-parse-url' app.setAppLogsPath() enableSourceMaps() From 855a576f2e456f0ed18a9750544a12f5f96097f0 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 11:52:17 +0100 Subject: [PATCH 08/17] Always create a new instance or else open will just focus the app --- app/src/cli/main.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/cli/main.ts b/app/src/cli/main.ts index 66b0199f5c2..db9a3bc1236 100644 --- a/app/src/cli/main.ts +++ b/app/src/cli/main.ts @@ -5,7 +5,7 @@ import { spawn } from 'child_process' const _spawn = (args: string[]) => { if (process.platform === 'darwin') { const execPath = join(__dirname, '../../../').replace(/\/$/, '') - return spawn('open', [execPath, '--args', ...args], { + return spawn('open', ['-n', execPath, '--args', ...args], { stdio: ['ignore', 'inherit', 'inherit'], }) } else if (process.platform === 'win32') { From 06542771a8acc48261ee8a942ca5c93339ee03eb Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 13:05:48 +0100 Subject: [PATCH 09/17] First take on Windows CLI support --- app/src/cli/main.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/src/cli/main.ts b/app/src/cli/main.ts index db9a3bc1236..900fa0233c1 100644 --- a/app/src/cli/main.ts +++ b/app/src/cli/main.ts @@ -2,17 +2,20 @@ import { join, resolve } from 'path' import parse from 'minimist' import { spawn } from 'child_process' -const _spawn = (args: string[]) => { +const _spawn = (cliArgs: string[]) => { + let execPath + let args = cliArgs if (process.platform === 'darwin') { - const execPath = join(__dirname, '../../../').replace(/\/$/, '') - return spawn('open', ['-n', execPath, '--args', ...args], { - stdio: ['ignore', 'inherit', 'inherit'], - }) + execPath = 'open' + const desktopPath = join(__dirname, '../../../').replace(/\/$/, '') + args = ['-n', desktopPath, '--args', ...cliArgs] } else if (process.platform === 'win32') { - throw new Error('TODO') + execPath = join(__dirname, '../../../GitHubDesktop.exe') } else { throw new Error('Unsupported platform') } + + return spawn(execPath, args, { stdio: 'inherit' }) } const run = (...args: Array) => From 5d3446eb5d962878e2afa47f6f7a847ef236ef58 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 15:45:07 +0100 Subject: [PATCH 10/17] Get windows CLI working --- app/src/cli/main.ts | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/app/src/cli/main.ts b/app/src/cli/main.ts index 900fa0233c1..446c651546d 100644 --- a/app/src/cli/main.ts +++ b/app/src/cli/main.ts @@ -1,32 +1,30 @@ import { join, resolve } from 'path' import parse from 'minimist' -import { spawn } from 'child_process' +import { execFile } from 'child_process' -const _spawn = (cliArgs: string[]) => { - let execPath - let args = cliArgs +const getExecParams = (args: string[]) => { if (process.platform === 'darwin') { - execPath = 'open' - const desktopPath = join(__dirname, '../../../').replace(/\/$/, '') - args = ['-n', desktopPath, '--args', ...cliArgs] + return { + command: 'open', + args: ['-n', join(__dirname, '../../..'), '--args', ...args], + } } else if (process.platform === 'win32') { - execPath = join(__dirname, '../../../GitHubDesktop.exe') - } else { - throw new Error('Unsupported platform') + const exeName = `GitHubDesktop${__DEV__ ? '-dev' : ''}.exe` + return { command: join(__dirname, `../../${exeName}`), args } } - - return spawn(execPath, args, { stdio: 'inherit' }) + throw new Error('Unsupported platform') } -const run = (...args: Array) => - _spawn(args.filter(x => typeof x === 'string')) - .on('error', e => { - console.error('Error running command', e) - process.exitCode = 1 - }) - .on('exit', code => { - process.exitCode = typeof code === 'number' ? code : process.exitCode - }) +const run = (...args: Array) => { + const p = getExecParams(args.filter(x => typeof x === 'string')) + return execFile(p.command, p.args, (error, stderr) => { + if (error) { + console.error(`Error running command ${p.command} ${p.args}`) + console.error(stderr) + process.exit(typeof error.code === 'number' ? error.code : 1) + } + }) +} const args = parse(process.argv.slice(2), { alias: { help: 'h', branch: 'b' }, From 35be3f7a8d822f7b477042cf381ae4a9d72aba23 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 15:48:00 +0100 Subject: [PATCH 11/17] Formatting --- app/src/cli/main.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/src/cli/main.ts b/app/src/cli/main.ts index 446c651546d..2e34114ab85 100644 --- a/app/src/cli/main.ts +++ b/app/src/cli/main.ts @@ -35,11 +35,11 @@ const args = parse(process.argv.slice(2), { const usage = (exitCode = 1): never => { process.stderr.write( 'GitHub Desktop CLI usage: \n' + - ' github Open the current directory\n' + - ' github open [path] Open the provided path\n' + - ' github clone [-b branch] Clone the repository by url or \n' + - ' name/owner (ex torvalds/linux),\n' + - ' optionally checking out the branch\n' + ' github Open the current directory\n' + + ' github open [path] Open the provided path\n' + + ' github clone [-b branch] Clone the repository by url or name/owner\n' + + ' (ex torvalds/linux), optionally checking out\n' + + ' the branch\n' ) process.exit(exitCode) } From b33cbb4efea21a190251ff1ec4f32e99fe7bfd65 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 15:49:42 +0100 Subject: [PATCH 12/17] Don't need this --- app/src/cli/main.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/cli/main.ts b/app/src/cli/main.ts index 2e34114ab85..6553915f589 100644 --- a/app/src/cli/main.ts +++ b/app/src/cli/main.ts @@ -29,7 +29,6 @@ const run = (...args: Array) => { const args = parse(process.argv.slice(2), { alias: { help: 'h', branch: 'b' }, boolean: ['help'], - string: ['branch'], }) const usage = (exitCode = 1): never => { From 462e28be6f111f29384a7f2fdc1477995e36bb66 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Wed, 11 Dec 2024 16:39:03 +0100 Subject: [PATCH 13/17] :art: cleanup --- app/src/cli/main.ts | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/app/src/cli/main.ts b/app/src/cli/main.ts index 6553915f589..48ecd76622d 100644 --- a/app/src/cli/main.ts +++ b/app/src/cli/main.ts @@ -1,29 +1,24 @@ import { join, resolve } from 'path' import parse from 'minimist' -import { execFile } from 'child_process' +import { execFile, ExecFileException } from 'child_process' -const getExecParams = (args: string[]) => { - if (process.platform === 'darwin') { - return { - command: 'open', - args: ['-n', join(__dirname, '../../..'), '--args', ...args], +const run = (...args: Array) => { + function cb(e: ExecFileException | null, stderr: string) { + if (e) { + console.error(`Error running command ${args}`) + console.error(stderr) + process.exit(e.code) } + } + + if (process.platform === 'darwin') { + execFile('open', ['-n', join(__dirname, '../../..'), '--args', ...args], cb) } else if (process.platform === 'win32') { const exeName = `GitHubDesktop${__DEV__ ? '-dev' : ''}.exe` - return { command: join(__dirname, `../../${exeName}`), args } + execFile(join(__dirname, `../../${exeName}`), args, cb) + } else { + throw new Error('Unsupported platform') } - throw new Error('Unsupported platform') -} - -const run = (...args: Array) => { - const p = getExecParams(args.filter(x => typeof x === 'string')) - return execFile(p.command, p.args, (error, stderr) => { - if (error) { - console.error(`Error running command ${p.command} ${p.args}`) - console.error(stderr) - process.exit(typeof error.code === 'number' ? error.code : 1) - } - }) } const args = parse(process.argv.slice(2), { @@ -57,8 +52,10 @@ if (args.help || args._.at(0) === 'help') { if (!url) { usage(1) + } else if (typeof args.branch === 'string') { + run(`--cli-clone=${url}`, `--cli-branch=${args.branch}`) } else { - run(`--cli-clone=${url}`, args.branch && `--cli-branch=${args.branch}`) + run(`--cli-clone=${url}`) } } else { const [firstArg, secondArg] = args._ From 42a20e4f351da4a2ad30e3f92c019a5fab9e16cc Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 12 Dec 2024 09:10:48 +0100 Subject: [PATCH 14/17] Error handling --- app/src/ui/index.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/src/ui/index.tsx b/app/src/ui/index.tsx index a74a57ad3b3..2ca1a0a007f 100644 --- a/app/src/ui/index.tsx +++ b/app/src/ui/index.tsx @@ -349,12 +349,16 @@ ipcRenderer.on('blur', () => { }) ipcRenderer.on('url-action', (_, action) => - dispatcher.dispatchURLAction(action) + dispatcher + .dispatchURLAction(action) + .catch(e => log.error(`CLI action ${action.name} failed`, e)) ) -ipcRenderer.on('cli-action', (_, action) => { - dispatcher.dispatchCLIAction(action) -}) +ipcRenderer.on('cli-action', (_, action) => + dispatcher + .dispatchCLIAction(action) + .catch(e => log.error(`CLI action ${action.kind} failed`, e)) +) // react-virtualized will use the literal string "grid" as the 'aria-label' // attribute unless we override it. This is a problem because aria-label should From b28fc490611e4daa259af21fdb03cf828bc95124 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 12 Dec 2024 09:11:14 +0100 Subject: [PATCH 15/17] typo --- app/src/ui/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/ui/index.tsx b/app/src/ui/index.tsx index 2ca1a0a007f..ad32d7dc7d7 100644 --- a/app/src/ui/index.tsx +++ b/app/src/ui/index.tsx @@ -351,7 +351,7 @@ ipcRenderer.on('blur', () => { ipcRenderer.on('url-action', (_, action) => dispatcher .dispatchURLAction(action) - .catch(e => log.error(`CLI action ${action.name} failed`, e)) + .catch(e => log.error(`URL action ${action.name} failed`, e)) ) ipcRenderer.on('cli-action', (_, action) => From 0410d1f657b8b31df99ff0338d10b69007b54740 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 12 Dec 2024 14:27:01 +0100 Subject: [PATCH 16/17] Don't need this any more --- app/src/lib/globals.d.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/lib/globals.d.ts b/app/src/lib/globals.d.ts index e1d4aa4a046..78dfcf54a76 100644 --- a/app/src/lib/globals.d.ts +++ b/app/src/lib/globals.d.ts @@ -44,7 +44,6 @@ declare const __RELEASE_CHANNEL__: | 'test' | 'development' -declare const __CLI_COMMANDS__: ReadonlyArray /** The URL for Squirrel's updates. */ declare const __UPDATES_URL__: string From 108dbfbf1c719362144706cbe77ff024c9fa26a0 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Tue, 17 Dec 2024 14:06:30 +0100 Subject: [PATCH 17/17] Update globals.d.ts --- app/src/lib/globals.d.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/lib/globals.d.ts b/app/src/lib/globals.d.ts index 78dfcf54a76..7c065f2ed85 100644 --- a/app/src/lib/globals.d.ts +++ b/app/src/lib/globals.d.ts @@ -44,7 +44,6 @@ declare const __RELEASE_CHANNEL__: | 'test' | 'development' - /** The URL for Squirrel's updates. */ declare const __UPDATES_URL__: string