From 4573cc289d32d0c78f3faf8c2eb7a753a94f8504 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 10:18:34 -0300 Subject: [PATCH 01/14] add helper function for detecting we're in a flatpak build --- app/src/lib/helpers/linux.ts | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 app/src/lib/helpers/linux.ts diff --git a/app/src/lib/helpers/linux.ts b/app/src/lib/helpers/linux.ts new file mode 100644 index 00000000000..8d4705a9c7a --- /dev/null +++ b/app/src/lib/helpers/linux.ts @@ -0,0 +1,3 @@ +export function isFlatpakBuild() { + return __LINUX__ && process.env.FLATPAK_HOST === '1' +} From b6f6e3a50a3280939d3ae39abf8b9e89e0847ca1 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 10:18:50 -0300 Subject: [PATCH 02/14] add docs for the useShell parameter --- app/src/lib/editors/found-editor.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/src/lib/editors/found-editor.ts b/app/src/lib/editors/found-editor.ts index a2c7f2bb5ad..c6a54597cc5 100644 --- a/app/src/lib/editors/found-editor.ts +++ b/app/src/lib/editors/found-editor.ts @@ -1,5 +1,11 @@ export interface IFoundEditor { readonly editor: T readonly path: string + /** + * Indicate to Desktop to launch the editor with the `shell: true` option included. + * + * This is available to all platforms, but is only currently used by some Windows + * editors as their launch programs end in `.cmd` + */ readonly usesShell?: boolean } From da9bf483dfc3a8fcae2229fc40d0d4dfcfee670d Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 10:38:08 -0300 Subject: [PATCH 03/14] add helpers for flatpak interactions with shells --- app/src/lib/helpers/linux.ts | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/app/src/lib/helpers/linux.ts b/app/src/lib/helpers/linux.ts index 8d4705a9c7a..f67a00c7368 100644 --- a/app/src/lib/helpers/linux.ts +++ b/app/src/lib/helpers/linux.ts @@ -1,3 +1,57 @@ +import { join } from 'path' +import { pathExists as pathExistsInternal } from 'fs-extra' +import { spawn, SpawnOptionsWithoutStdio } from 'child_process' + export function isFlatpakBuild() { return __LINUX__ && process.env.FLATPAK_HOST === '1' } + +/** + * Convert an executable path to be relative to the flatpak host + * + * @param path a path to an executable relative to the root of the filesystem + * + */ +function convertToFlatpakPath(path: string) { + return join('/var/run/host', path) +} + +/** + * Checks the file path on disk exists before attempting to launch a specific shell + * + * @param path + * + * @returns `true` if the path can be resolved, or `false` otherwise + */ +export async function pathExists(path: string): Promise { + if (isFlatpakBuild()) { + path = convertToFlatpakPath(path) + } + + try { + return await pathExistsInternal(path) + } catch { + return false + } +} + +/** + * Spawn a particular shell in a way that works for Flatpak-based usage + * + * @param path path to shell, relative to the root of the filesystem + * @param args arguments to provide to the shell + * @param options additional options to provide to spawn + * + * @returns a child process to observe and monitor + */ +export function spawnShell( + path: string, + args: string[], + options?: SpawnOptionsWithoutStdio +) { + if (isFlatpakBuild()) { + return spawn('flatpak-spawn', ['--host', path, ...args], options) + } + + return spawn(path, args, options) +} From b435a98b69f0a7d3dd64c48f44829bba0eab5f50 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 10:38:43 -0300 Subject: [PATCH 04/14] update shells to use new API on Linux --- app/src/lib/shells/linux.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/app/src/lib/shells/linux.ts b/app/src/lib/shells/linux.ts index 47a5c7f038c..393e0bb7e30 100644 --- a/app/src/lib/shells/linux.ts +++ b/app/src/lib/shells/linux.ts @@ -1,8 +1,8 @@ -import { spawn, ChildProcess } from 'child_process' -import { pathExists } from 'fs-extra' +import { ChildProcess } from 'child_process' import { assertNever } from '../fatal-error' import { IFoundShell } from './found-shell' import { parseEnumValue } from '../enum' +import { pathExists as pathExistsLinux, spawnShell } from '../helpers/linux' export enum Shell { Gnome = 'GNOME Terminal', @@ -27,7 +27,7 @@ export function parse(label: string): Shell { } async function getPathIfAvailable(path: string): Promise { - return (await pathExists(path)) ? path : null + return (await pathExistsLinux(path)) ? path : null } function getShellPath(shell: Shell): Promise { @@ -164,21 +164,25 @@ export function launch( case Shell.Terminator: case Shell.XFCE: case Shell.Alacritty: - return spawn(foundShell.path, ['--working-directory', path]) + return spawnShell(foundShell.path, ['--working-directory', path]) case Shell.Urxvt: - return spawn(foundShell.path, ['-cd', path]) + return spawnShell(foundShell.path, ['-cd', path]) case Shell.Konsole: - return spawn(foundShell.path, ['--workdir', path]) + return spawnShell(foundShell.path, ['--workdir', path]) case Shell.Xterm: - return spawn(foundShell.path, ['-e', '/bin/bash'], { cwd: path }) + return spawnShell(foundShell.path, ['-e', '/bin/bash'], { cwd: path }) case Shell.Terminology: - return spawn(foundShell.path, ['-d', path]) + return spawnShell(foundShell.path, ['-d', path]) case Shell.Deepin: - return spawn(foundShell.path, ['-w', path]) + return spawnShell(foundShell.path, ['-w', path]) case Shell.Elementary: - return spawn(foundShell.path, ['-w', path]) + return spawnShell(foundShell.path, ['-w', path]) case Shell.Kitty: - return spawn(foundShell.path, ['--single-instance', '--directory', path]) + return spawnShell(foundShell.path, [ + '--single-instance', + '--directory', + path, + ]) default: return assertNever(shell, `Unknown shell: ${shell}`) } From 85a28b806b97e2dc661292765ac6787cf92d8113 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 10:44:50 -0300 Subject: [PATCH 05/14] add helper method for spawning editor --- app/src/lib/helpers/linux.ts | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/app/src/lib/helpers/linux.ts b/app/src/lib/helpers/linux.ts index f67a00c7368..8c402247a87 100644 --- a/app/src/lib/helpers/linux.ts +++ b/app/src/lib/helpers/linux.ts @@ -1,6 +1,11 @@ import { join } from 'path' import { pathExists as pathExistsInternal } from 'fs-extra' -import { spawn, SpawnOptionsWithoutStdio } from 'child_process' +import { + ChildProcess, + spawn, + SpawnOptionsWithoutStdio, + SpawnOptions, +} from 'child_process' export function isFlatpakBuild() { return __LINUX__ && process.env.FLATPAK_HOST === '1' @@ -40,7 +45,7 @@ export async function pathExists(path: string): Promise { * * @param path path to shell, relative to the root of the filesystem * @param args arguments to provide to the shell - * @param options additional options to provide to spawn + * @param options additional options to provide to spawn function * * @returns a child process to observe and monitor */ @@ -48,10 +53,33 @@ export function spawnShell( path: string, args: string[], options?: SpawnOptionsWithoutStdio -) { +): ChildProcess { if (isFlatpakBuild()) { return spawn('flatpak-spawn', ['--host', path, ...args], options) } return spawn(path, args, options) } + +/** + * Spawn a given editor in a way that works for Flatpak-based usage + * + * @param path path to editor, relative to the root of the filesystem + * @param workingDirectory working directory to open initially in editor + * @param options additional options to provide to spawn function + */ +export function spawnEditor( + path: string, + workingDirectory: string, + options: SpawnOptions +): ChildProcess { + if (isFlatpakBuild()) { + return spawn( + 'flatpak-spawn', + ['--host', `"${path}"`, `"${workingDirectory}"`], + options + ) + } else { + return spawn(path, [workingDirectory], options) + } +} From a3e2d0b45bf08fad1d3f324e4cdba422ffcc04b2 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 10:49:32 -0300 Subject: [PATCH 06/14] update editor to use new abstractions on Linux --- app/src/lib/editors/launch.ts | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/app/src/lib/editors/launch.ts b/app/src/lib/editors/launch.ts index 1d098f3843f..d20be766e48 100644 --- a/app/src/lib/editors/launch.ts +++ b/app/src/lib/editors/launch.ts @@ -1,7 +1,25 @@ import { spawn, SpawnOptions } from 'child_process' -import { pathExists } from 'fs-extra' +import { pathExists as pathExistsDefault } from 'fs-extra' +import { pathExists as pathExistsLinux, spawnEditor } from '../helpers/linux' import { ExternalEditorError, FoundEditor } from './shared' +/** + * Use a platform-specific pathExists based on the platform, to simplify changes + * to the application logic + * + * @param path the location of some program on disk + * + * @returns `true` if the path exists on disk, or `false` otherwise + * + */ +function pathExists(path: string) { + if (__LINUX__) { + return pathExistsLinux(path) + } else { + return pathExistsDefault(path) + } +} + /** * Open a given file or folder in the desired external editor. * @@ -35,6 +53,8 @@ export async function launchExternalEditor( // In macOS we can use `open`, which will open the right executable file // for us, we only need the path to the editor .app folder. spawn('open', ['-a', editorPath, fullPath], opts) + } else if (__LINUX__) { + spawnEditor(editorPath, fullPath, opts) } else { spawn(editorPath, [fullPath], opts) } From f46a87b6b9ede3268c2876b6a6b47a77d6871f7b Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 11:37:29 -0300 Subject: [PATCH 07/14] avoid conversion for /opt paths --- app/src/lib/helpers/linux.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/lib/helpers/linux.ts b/app/src/lib/helpers/linux.ts index 8c402247a87..235474c0342 100644 --- a/app/src/lib/helpers/linux.ts +++ b/app/src/lib/helpers/linux.ts @@ -17,7 +17,11 @@ export function isFlatpakBuild() { * @param path a path to an executable relative to the root of the filesystem * */ -function convertToFlatpakPath(path: string) { +export function convertToFlatpakPath(path: string) { + if (path.startsWith('/opt/')) { + return path + } + return join('/var/run/host', path) } From 2846fd29db0955ddeb80c1108337b2b802b52144 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 11:37:47 -0300 Subject: [PATCH 08/14] add tests for specific cases with convertToFlatpakPath --- app/test/unit/helpers/linux-test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 app/test/unit/helpers/linux-test.ts diff --git a/app/test/unit/helpers/linux-test.ts b/app/test/unit/helpers/linux-test.ts new file mode 100644 index 00000000000..0971871e6d4 --- /dev/null +++ b/app/test/unit/helpers/linux-test.ts @@ -0,0 +1,14 @@ +import { convertToFlatpakPath } from '../../../src/lib/helpers/linux' + +describe('convertToFlatpakPath()', () => { + it('converts /usr paths', () => { + const path = '/usr/bin/subl' + const expectedPath = '/var/run/host/usr/bin/subl' + expect(convertToFlatpakPath(path)).toEqual(expectedPath) + }) + + it('preserves /opt paths', () => { + const path = '/opt/slickedit-pro2018/bin/vs' + expect(convertToFlatpakPath(path)).toEqual(path) + }) +}) From abc817faaf17b6b04581c57b91ea798de399fe14 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 17:08:41 -0300 Subject: [PATCH 09/14] move tests into platform-specific global --- app/test/unit/helpers/linux-test.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/app/test/unit/helpers/linux-test.ts b/app/test/unit/helpers/linux-test.ts index 0971871e6d4..bf36b4a5cae 100644 --- a/app/test/unit/helpers/linux-test.ts +++ b/app/test/unit/helpers/linux-test.ts @@ -1,14 +1,16 @@ import { convertToFlatpakPath } from '../../../src/lib/helpers/linux' -describe('convertToFlatpakPath()', () => { - it('converts /usr paths', () => { - const path = '/usr/bin/subl' - const expectedPath = '/var/run/host/usr/bin/subl' - expect(convertToFlatpakPath(path)).toEqual(expectedPath) - }) +if (__LINUX__) { + describe('convertToFlatpakPath()', () => { + it('converts /usr paths', () => { + const path = '/usr/bin/subl' + const expectedPath = '/var/run/host/usr/bin/subl' + expect(convertToFlatpakPath(path)).toEqual(expectedPath) + }) - it('preserves /opt paths', () => { - const path = '/opt/slickedit-pro2018/bin/vs' - expect(convertToFlatpakPath(path)).toEqual(path) + it('preserves /opt paths', () => { + const path = '/opt/slickedit-pro2018/bin/vs' + expect(convertToFlatpakPath(path)).toEqual(path) + }) }) -}) +} From d9793bec539f7886a293ff2d25d7638a27a290f7 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 17:20:53 -0300 Subject: [PATCH 10/14] add new path to vscodium Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com> --- app/src/lib/editors/linux.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/lib/editors/linux.ts b/app/src/lib/editors/linux.ts index 91fd4ecbe14..a8b1a984401 100644 --- a/app/src/lib/editors/linux.ts +++ b/app/src/lib/editors/linux.ts @@ -34,7 +34,11 @@ const editors: ILinuxExternalEditor[] = [ }, { name: 'VSCodium', - paths: ['/usr/bin/codium', '/var/lib/flatpak/app/com.vscodium.codium'], + paths: [ + '/usr/bin/codium', + '/var/lib/flatpak/app/com.vscodium.codium', + '/usr/share/vscodium-bin/bin/codium', + ], }, { name: 'Sublime Text', From 3bbef9040b07debcb76935bb257e7d0ccf71e12e Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 17:21:12 -0300 Subject: [PATCH 11/14] ensure pathExists for shell is flatpak-aware Co-Authored-By: nullrequest <30698906+advaithm@users.noreply.github.com> --- app/src/lib/shells/shared.ts | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/app/src/lib/shells/shared.ts b/app/src/lib/shells/shared.ts index f09bfdfa774..4ec05019ac8 100644 --- a/app/src/lib/shells/shared.ts +++ b/app/src/lib/shells/shared.ts @@ -1,9 +1,10 @@ import { ChildProcess } from 'child_process' -import { pathExists } from 'fs-extra' +import { pathExists as pathExistsDefault } from 'fs-extra' import * as Darwin from './darwin' import * as Win32 from './win32' import * as Linux from './linux' +import { pathExists as pathExistsLinux } from '../helpers/linux' import { IFoundShell } from './found-shell' import { ShellError } from './error' @@ -72,6 +73,23 @@ export async function findShellOrDefault(shell: Shell): Promise { } } +/** + * Use a platform-specific pathExists based on the platform, to simplify changes + * to the application logic + * + * @param path the location of some program on disk + * + * @returns `true` if the path exists on disk, or `false` otherwise + * + */ +function pathExists(path: string) { + if (__LINUX__) { + return pathExistsLinux(path) + } else { + return pathExistsDefault(path) + } +} + /** Launch the given shell at the path. */ export async function launchShell( shell: FoundShell, From bdbb31acc84356c25f0fb97ab192fc2af9010f37 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 13 Jul 2021 17:34:47 -0300 Subject: [PATCH 12/14] add stub tests for mac/windows to get CI passing --- app/src/lib/helpers/linux.ts | 4 ++++ app/test/unit/helpers/linux-test.ts | 22 ++++++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/src/lib/helpers/linux.ts b/app/src/lib/helpers/linux.ts index 235474c0342..a112a6ceda1 100644 --- a/app/src/lib/helpers/linux.ts +++ b/app/src/lib/helpers/linux.ts @@ -18,6 +18,10 @@ export function isFlatpakBuild() { * */ export function convertToFlatpakPath(path: string) { + if (!__LINUX__) { + return path + } + if (path.startsWith('/opt/')) { return path } diff --git a/app/test/unit/helpers/linux-test.ts b/app/test/unit/helpers/linux-test.ts index bf36b4a5cae..df18564e441 100644 --- a/app/test/unit/helpers/linux-test.ts +++ b/app/test/unit/helpers/linux-test.ts @@ -1,7 +1,7 @@ import { convertToFlatpakPath } from '../../../src/lib/helpers/linux' -if (__LINUX__) { - describe('convertToFlatpakPath()', () => { +describe('convertToFlatpakPath()', () => { + if (__LINUX__) { it('converts /usr paths', () => { const path = '/usr/bin/subl' const expectedPath = '/var/run/host/usr/bin/subl' @@ -12,5 +12,19 @@ if (__LINUX__) { const path = '/opt/slickedit-pro2018/bin/vs' expect(convertToFlatpakPath(path)).toEqual(path) }) - }) -} + } + + if (__WIN32__) { + it('returns same path', () => { + const path = 'C:\\Windows\\System32\\Notepad.exe' + expect(convertToFlatpakPath(path)).toEqual(path) + }) + } + + if (__DARWIN__) { + it('returns same path', () => { + const path = '/usr/local/bin/code' + expect(convertToFlatpakPath(path)).toEqual(path) + }) + } +}) From 5095b7018888d8b46717c21d72ae49720cd9570d Mon Sep 17 00:00:00 2001 From: advaithm Date: Wed, 14 Jul 2021 08:42:22 +0530 Subject: [PATCH 13/14] fixed code editor detection --- app/src/lib/editors/linux.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/lib/editors/linux.ts b/app/src/lib/editors/linux.ts index a8b1a984401..04c9fb4e45a 100644 --- a/app/src/lib/editors/linux.ts +++ b/app/src/lib/editors/linux.ts @@ -1,4 +1,4 @@ -import { pathExists } from 'fs-extra' +import { pathExists } from '../helpers/linux' import { IFoundEditor } from './found-editor' From 06c74810a9727b7f22fda086d60f9fba60e1898c Mon Sep 17 00:00:00 2001 From: advaithm Date: Wed, 14 Jul 2021 11:30:55 +0530 Subject: [PATCH 14/14] code editor launched,but path appends " at the end --- app/src/lib/helpers/linux.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/lib/helpers/linux.ts b/app/src/lib/helpers/linux.ts index a112a6ceda1..73fe58887fe 100644 --- a/app/src/lib/helpers/linux.ts +++ b/app/src/lib/helpers/linux.ts @@ -84,7 +84,7 @@ export function spawnEditor( if (isFlatpakBuild()) { return spawn( 'flatpak-spawn', - ['--host', `"${path}"`, `"${workingDirectory}"`], + ['--host', path, `"${workingDirectory}"`], options ) } else {