Skip to content

Commit d6c10a2

Browse files
authored
Merge pull request desktop#18954 from desktop/improved-integrations
Add support for custom shells/editors
2 parents bb31626 + 148e313 commit d6c10a2

29 files changed

+1833
-84
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ coverage/
55
npm-debug.log
66
yarn-error.log
77
app/node_modules/
8+
vendor/windows-argv-parser/build/
89
.DS_Store
910
.awcache
1011
.idea/

app/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,15 @@
5757
"react-virtualized": "^9.20.0",
5858
"registry-js": "^1.16.0",
5959
"source-map-support": "^0.4.15",
60+
"string-argv": "^0.3.2",
6061
"strip-ansi": "^4.0.0",
6162
"textarea-caret": "^3.0.2",
6263
"triple-beam": "^1.3.0",
6364
"tslib": "^2.0.0",
6465
"untildify": "^3.0.2",
6566
"username": "^5.1.0",
6667
"uuid": "^3.0.1",
68+
"windows-argv-parser": "file:../vendor/windows-argv-parser",
6769
"winston": "^3.6.0"
6870
},
6971
"devDependencies": {

app/src/lib/app-state.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import { IChangesetData } from './git'
4848
import { Popup } from '../models/popup'
4949
import { RepoRulesInfo } from '../models/repo-rules'
5050
import { IAPIRepoRuleset } from './api'
51+
import { ICustomIntegration } from './custom-integration'
5152

5253
export enum SelectionType {
5354
Repository,
@@ -323,6 +324,18 @@ export interface IAppState {
323324
*/
324325
readonly lastThankYou: ILastThankYou | undefined
325326

327+
/** Whether or not the user wants to use a custom editor. */
328+
readonly useCustomEditor: boolean
329+
330+
/** Info needed to launch a custom editor chosen by the user. */
331+
readonly customEditor: ICustomIntegration | null
332+
333+
/** Whether or not the user wants to use a custom shell. */
334+
readonly useCustomShell: boolean
335+
336+
/** Info needed to launch a custom shell chosen by the user. */
337+
readonly customShell: ICustomIntegration | null
338+
326339
/**
327340
* Whether or not the CI status popover is visible.
328341
*/

app/src/lib/custom-integration.ts

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import { parseCommandLineArgv } from 'windows-argv-parser'
2+
import stringArgv from 'string-argv'
3+
import { promisify } from 'util'
4+
import { exec } from 'child_process'
5+
import { access, stat } from 'fs/promises'
6+
import * as fs from 'fs'
7+
8+
const execAsync = promisify(exec)
9+
10+
/** The string that will be replaced by the target path in the custom integration arguments */
11+
export const TargetPathArgument = '%TARGET_PATH%'
12+
13+
/** The interface representing a custom integration (external editor or shell) */
14+
export interface ICustomIntegration {
15+
/** The path to the custom integration */
16+
readonly path: string
17+
/** The arguments to pass to the custom integration */
18+
readonly arguments: ReadonlyArray<string>
19+
/** The bundle ID of the custom integration (macOS only) */
20+
readonly bundleID?: string
21+
}
22+
23+
/**
24+
* Parse the arguments string of a custom integration into an array of strings.
25+
*
26+
* @param args The arguments string to parse
27+
*/
28+
export function parseCustomIntegrationArguments(
29+
args: string
30+
): ReadonlyArray<string> {
31+
return __WIN32__ ? parseCommandLineArgv(args) : stringArgv(args)
32+
}
33+
34+
// Function to retrieve, on macOS, the bundleId of an app given its path
35+
async function getAppBundleID(path: string) {
36+
try {
37+
// Ensure the path ends with `.app` for applications
38+
if (!path.endsWith('.app')) {
39+
throw new Error(
40+
'The provided path does not point to a macOS application.'
41+
)
42+
}
43+
44+
// Use mdls to query the kMDItemCFBundleIdentifier attribute
45+
const { stdout } = await execAsync(
46+
`mdls -name kMDItemCFBundleIdentifier -raw "${path}"`
47+
)
48+
const bundleId = stdout.trim()
49+
50+
// Check for valid output
51+
if (!bundleId || bundleId === '(null)') {
52+
return undefined
53+
}
54+
55+
return bundleId
56+
} catch (error) {
57+
console.error('Failed to retrieve bundle ID:', error)
58+
return undefined
59+
}
60+
}
61+
62+
/**
63+
* Replace the target path placeholder in the custom integration arguments with
64+
* the actual target path.
65+
*
66+
* @param args The custom integration arguments
67+
* @param repoPath The target path to replace the placeholder with
68+
*/
69+
export function expandTargetPathArgument(
70+
args: ReadonlyArray<string>,
71+
repoPath: string
72+
): ReadonlyArray<string> {
73+
return args.map(arg => arg.replaceAll(TargetPathArgument, repoPath))
74+
}
75+
76+
/**
77+
* Check if the custom integration arguments contain the target path placeholder.
78+
*
79+
* @param args The custom integration arguments
80+
*/
81+
export function checkTargetPathArgument(args: ReadonlyArray<string>): boolean {
82+
return args.some(arg => arg.includes(TargetPathArgument))
83+
}
84+
85+
/**
86+
* Validate the path of a custom integration.
87+
*
88+
* @param path The path to the custom integration
89+
*
90+
* @returns An object with a boolean indicating if the path is valid and, if
91+
* the path is a macOS app, the bundle ID of the app.
92+
*/
93+
export async function validateCustomIntegrationPath(
94+
path: string
95+
): Promise<{ isValid: boolean; bundleID?: string }> {
96+
if (path.length === 0) {
97+
return { isValid: false }
98+
}
99+
100+
let bundleID = undefined
101+
102+
try {
103+
const pathStat = await stat(path)
104+
const canBeExecuted = await access(path, fs.constants.X_OK)
105+
.then(() => true)
106+
.catch(() => false)
107+
108+
const isExecutableFile = pathStat.isFile() && canBeExecuted
109+
110+
// On macOS, not only executable files are valid, but also apps (which are
111+
// directories with a `.app` extension and from which we can retrieve
112+
// the app bundle ID)
113+
if (__DARWIN__ && !isExecutableFile && pathStat.isDirectory()) {
114+
bundleID = await getAppBundleID(path)
115+
}
116+
117+
return { isValid: isExecutableFile || !!bundleID, bundleID }
118+
} catch (e) {
119+
return { isValid: false }
120+
}
121+
}
122+
123+
/**
124+
* Check if a custom integration is valid (meaning both the path and the
125+
* arguments are valid).
126+
*
127+
* @param customIntegration The custom integration to validate
128+
*/
129+
export async function isValidCustomIntegration(
130+
customIntegration: ICustomIntegration
131+
): Promise<boolean> {
132+
const pathResult = await validateCustomIntegrationPath(customIntegration.path)
133+
const targetPathPresent = checkTargetPathArgument(customIntegration.arguments)
134+
return pathResult.isValid && targetPathPresent
135+
}

app/src/lib/editors/launch.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { spawn, SpawnOptions } from 'child_process'
22
import { pathExists } from '../../ui/lib/path-exists'
33
import { ExternalEditorError, FoundEditor } from './shared'
4+
import {
5+
expandTargetPathArgument,
6+
ICustomIntegration,
7+
} from '../custom-integration'
48

59
/**
610
* Open a given file or folder in the desired external editor.
@@ -54,3 +58,68 @@ export async function launchExternalEditor(
5458
}
5559
}
5660
}
61+
62+
/**
63+
* Open a given file or folder in the desired custom external editor.
64+
*
65+
* @param fullPath A folder or file path to pass as an argument when launching the editor.
66+
* @param customEditor The external editor to launch.
67+
*/
68+
export async function launchCustomExternalEditor(
69+
fullPath: string,
70+
customEditor: ICustomIntegration
71+
): Promise<void> {
72+
const editorPath = customEditor.path
73+
const exists = await pathExists(editorPath)
74+
const label = __DARWIN__ ? 'Settings' : 'Options'
75+
if (!exists) {
76+
throw new ExternalEditorError(
77+
`Could not find executable for custom editor at path '${customEditor.path}'. Please open ${label} and select an available editor.`,
78+
{ openPreferences: true }
79+
)
80+
}
81+
82+
const opts: SpawnOptions = {
83+
// Make sure the editor processes are detached from the Desktop app.
84+
// Otherwise, some editors (like Notepad++) will be killed when the
85+
// Desktop app is closed.
86+
detached: true,
87+
}
88+
89+
// Replace instances of RepoPathArgument with fullPath in customEditor.arguments
90+
const args = expandTargetPathArgument(customEditor.arguments, fullPath)
91+
92+
try {
93+
// This logic around `usesShell` is also used in Windows `getAvailableEditors` implementation
94+
const usesShell = editorPath.endsWith('.cmd')
95+
if (usesShell) {
96+
spawn(`"${editorPath}"`, args, {
97+
...opts,
98+
shell: true,
99+
})
100+
} else if (__DARWIN__ && customEditor.bundleID) {
101+
// In macOS we can use `open` if it's an app (i.e. if we have a bundleID),
102+
// which will open the right executable file for us, we only need the path
103+
// to the editor .app folder.
104+
spawn('open', ['-a', editorPath, ...args], opts)
105+
} else {
106+
spawn(editorPath, args, opts)
107+
}
108+
} catch (error) {
109+
log.error(
110+
`Error while launching custom editor at path ${customEditor.path} with arguments ${args}`,
111+
error
112+
)
113+
if (error?.code === 'EACCES') {
114+
throw new ExternalEditorError(
115+
`GitHub Desktop doesn't have the proper permissions to start custom editor at path ${customEditor.path}. Please open ${label} and try another editor.`,
116+
{ openPreferences: true }
117+
)
118+
} else {
119+
throw new ExternalEditorError(
120+
`Something went wrong while trying to start custom editor at path ${customEditor.path}. Please open ${label} and try another editor.`,
121+
{ openPreferences: true }
122+
)
123+
}
124+
}
125+
}

app/src/lib/feature-flag.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,5 @@ export const enableLinkUnderlines = enableDiffCheckMarksAndLinkUnderlines
9898

9999
export const enableExternalCredentialHelper = () => true
100100
export const enableCredentialHelperTrampoline = () => true
101+
102+
export const enableCustomIntegration = enableDevelopmentFeatures

app/src/lib/shells/darwin.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import { assertNever } from '../fatal-error'
33
import appPath from 'app-path'
44
import { parseEnumValue } from '../enum'
55
import { FoundShell } from './shared'
6+
import {
7+
expandTargetPathArgument,
8+
ICustomIntegration,
9+
} from '../custom-integration'
610

711
export enum Shell {
812
Terminal = 'Terminal',
@@ -190,3 +194,14 @@ export function launch(
190194
return spawn('open', ['-b', foundShell.bundleID, path])
191195
}
192196
}
197+
198+
export function launchCustomShell(
199+
customShell: ICustomIntegration,
200+
path: string
201+
): ChildProcess {
202+
const args = expandTargetPathArgument(customShell.arguments, path)
203+
204+
return customShell.bundleID
205+
? spawn('open', ['-b', customShell.bundleID, ...args])
206+
: spawn(customShell.path, args)
207+
}

app/src/lib/shells/linux.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import { assertNever } from '../fatal-error'
33
import { parseEnumValue } from '../enum'
44
import { pathExists } from '../../ui/lib/path-exists'
55
import { FoundShell } from './shared'
6+
import {
7+
expandTargetPathArgument,
8+
ICustomIntegration,
9+
} from '../custom-integration'
610

711
export enum Shell {
812
Gnome = 'GNOME Terminal',
@@ -183,3 +187,11 @@ export function launch(
183187
return assertNever(shell, `Unknown shell: ${shell}`)
184188
}
185189
}
190+
191+
export function launchCustomShell(
192+
customShell: ICustomIntegration,
193+
path: string
194+
): ChildProcess {
195+
const args = expandTargetPathArgument(customShell.arguments, path)
196+
return spawn(customShell.path, args)
197+
}

app/src/lib/shells/shared.ts

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@ import * as Win32 from './win32'
55
import * as Linux from './linux'
66
import { ShellError } from './error'
77
import { pathExists } from '../../ui/lib/path-exists'
8+
import { ICustomIntegration } from '../custom-integration'
89

910
export type Shell = Darwin.Shell | Win32.Shell | Linux.Shell
1011

1112
export type FoundShell<T extends Shell> = {
1213
readonly shell: T
1314
readonly path: string
14-
readonly extraArgs?: string[]
15+
readonly extraArgs?: ReadonlyArray<string>
1516
} & (T extends Darwin.Shell
1617
? {
1718
readonly bundleID: string
@@ -120,8 +121,45 @@ export async function launchShell(
120121
}
121122
}
122123

124+
/** Launch custom shell at the path. */
125+
export async function launchCustomShell(
126+
customShell: ICustomIntegration,
127+
path: string,
128+
onError: (error: Error) => void
129+
): Promise<void> {
130+
// We have to manually cast the wider `Shell` type into the platform-specific
131+
// type. This is less than ideal, but maybe the best we can do without
132+
// platform-specific build targets.
133+
const exists = await pathExists(customShell.path)
134+
if (!exists) {
135+
const label = __DARWIN__ ? 'Settings' : 'Options'
136+
throw new ShellError(
137+
`Could not find executable for custom shell at path '${customShell.path}'. Please open ${label} and select an available shell.`
138+
)
139+
}
140+
141+
let cp: ChildProcess | null = null
142+
143+
if (__DARWIN__) {
144+
cp = Darwin.launchCustomShell(customShell, path)
145+
} else if (__WIN32__) {
146+
cp = Win32.launchCustomShell(customShell, path)
147+
} else if (__LINUX__) {
148+
cp = Linux.launchCustomShell(customShell, path)
149+
}
150+
151+
if (cp != null) {
152+
addErrorTracing('Custom Shell', cp, onError)
153+
return Promise.resolve()
154+
} else {
155+
return Promise.reject(
156+
`Platform not currently supported for launching shells: ${process.platform}`
157+
)
158+
}
159+
}
160+
123161
function addErrorTracing(
124-
shell: Shell,
162+
shell: Shell | 'Custom Shell',
125163
cp: ChildProcess,
126164
onError: (error: Error) => void
127165
) {

0 commit comments

Comments
 (0)