From ad345a777027c57795e61b99e32eabbea1e81d7b Mon Sep 17 00:00:00 2001 From: estib Date: Tue, 29 Oct 2024 10:39:23 +0100 Subject: [PATCH] fix: Open files on editor in Windows In order to open the files using the editor url, we need to use forward slashes for the path, even on Windows. --- .../ProjectSettingsMenuAction.svelte | 8 ++- .../src/lib/components/BoardEmptyState.svelte | 11 +-- .../src/lib/components/EditMode.svelte | 10 +-- .../src/lib/file/FileContextMenu.svelte | 11 +-- .../src/lib/hunk/HunkContextMenu.svelte | 11 +-- apps/desktop/src/lib/utils/url.test.ts | 70 ++++++++++++++++++- apps/desktop/src/lib/utils/url.ts | 29 ++++++++ 7 files changed, 130 insertions(+), 20 deletions(-) diff --git a/apps/desktop/src/lib/barmenuActions/ProjectSettingsMenuAction.svelte b/apps/desktop/src/lib/barmenuActions/ProjectSettingsMenuAction.svelte index ee2a194c4..c706c48b8 100644 --- a/apps/desktop/src/lib/barmenuActions/ProjectSettingsMenuAction.svelte +++ b/apps/desktop/src/lib/barmenuActions/ProjectSettingsMenuAction.svelte @@ -5,7 +5,7 @@ import { SETTINGS, type Settings } from '$lib/settings/userSettings'; import * as events from '$lib/utils/events'; import { unsubscribe } from '$lib/utils/unsubscribe'; - import { openExternalUrl } from '$lib/utils/url'; + import { getEditorUri, openExternalUrl } from '$lib/utils/url'; import { getContextStoreBySymbol } from '@gitbutler/shared/context'; import { getContext } from '@gitbutler/shared/context'; import { onMount } from 'svelte'; @@ -23,7 +23,11 @@ const unsubscribeopenInEditor = listen( 'menu://project/open-in-vscode/clicked', async () => { - const path = `${$userSettings.defaultCodeEditor.schemeIdentifer}://file${project.vscodePath}?windowId=_blank`; + const path = getEditorUri({ + schemeId: $userSettings.defaultCodeEditor.schemeIdentifer, + path: [project.vscodePath], + searchParams: { windowId: '_blank' } + }); openExternalUrl(path); } ); diff --git a/apps/desktop/src/lib/components/BoardEmptyState.svelte b/apps/desktop/src/lib/components/BoardEmptyState.svelte index cb55d8a01..61f75e807 100644 --- a/apps/desktop/src/lib/components/BoardEmptyState.svelte +++ b/apps/desktop/src/lib/components/BoardEmptyState.svelte @@ -4,7 +4,7 @@ import { BaseBranch } from '$lib/baseBranch/baseBranch'; import { getGitHost } from '$lib/gitHost/interface/gitHost'; import { SETTINGS, type Settings } from '$lib/settings/userSettings'; - import { openExternalUrl } from '$lib/utils/url'; + import { getEditorUri, openExternalUrl } from '$lib/utils/url'; import { BranchController } from '$lib/vbranches/branchController'; import { getContext, getContextStore, getContextStoreBySymbol } from '@gitbutler/shared/context'; import Icon from '@gitbutler/ui/Icon.svelte'; @@ -18,9 +18,12 @@ const project = getContext(Project); async function openInEditor() { - openExternalUrl( - `${$userSettings.defaultCodeEditor.schemeIdentifer}://file${project.vscodePath}/?windowId=_blank` - ); + const path = getEditorUri({ + schemeId: $userSettings.defaultCodeEditor.schemeIdentifer, + path: [project.vscodePath], + searchParams: { windowId: '_blank' } + }); + openExternalUrl(path); } diff --git a/apps/desktop/src/lib/components/EditMode.svelte b/apps/desktop/src/lib/components/EditMode.svelte index ce04393a2..2d46c6500 100644 --- a/apps/desktop/src/lib/components/EditMode.svelte +++ b/apps/desktop/src/lib/components/EditMode.svelte @@ -7,7 +7,7 @@ import ScrollableContainer from '$lib/scroll/ScrollableContainer.svelte'; import { SETTINGS, type Settings } from '$lib/settings/userSettings'; import { UncommitedFilesWatcher } from '$lib/uncommitedFiles/watcher'; - import { openExternalUrl } from '$lib/utils/url'; + import { getEditorUri, openExternalUrl } from '$lib/utils/url'; import { Commit, type RemoteFile } from '$lib/vbranches/types'; import { getContextStoreBySymbol } from '@gitbutler/shared/context'; import { getContext } from '@gitbutler/shared/context'; @@ -16,7 +16,6 @@ import InfoButton from '@gitbutler/ui/InfoButton.svelte'; import Avatar from '@gitbutler/ui/avatar/Avatar.svelte'; import FileListItem from '@gitbutler/ui/file/FileListItem.svelte'; - import { join } from '@tauri-apps/api/path'; import type { FileStatus } from '@gitbutler/ui/file/types'; import type { Writable } from 'svelte/store'; @@ -164,8 +163,11 @@ async function openAllConflictedFiles() { for (const file of conflictedFiles) { - const absPath = await join(project.vscodePath, file.path); - openExternalUrl(`${$userSettings.defaultCodeEditor.schemeIdentifer}://file${absPath}`); + const path = getEditorUri({ + schemeId: $userSettings.defaultCodeEditor.schemeIdentifer, + path: [project.vscodePath, file.path] + }); + openExternalUrl(path); } } diff --git a/apps/desktop/src/lib/file/FileContextMenu.svelte b/apps/desktop/src/lib/file/FileContextMenu.svelte index 5e2a17df0..2231357aa 100644 --- a/apps/desktop/src/lib/file/FileContextMenu.svelte +++ b/apps/desktop/src/lib/file/FileContextMenu.svelte @@ -6,7 +6,7 @@ import { SETTINGS, type Settings } from '$lib/settings/userSettings'; import { computeFileStatus } from '$lib/utils/fileStatus'; import * as toasts from '$lib/utils/toasts'; - import { openExternalUrl } from '$lib/utils/url'; + import { getEditorUri, openExternalUrl } from '$lib/utils/url'; import { BranchController } from '$lib/vbranches/branchController'; import { isAnyFile, LocalFile } from '$lib/vbranches/types'; import { getContextStoreBySymbol } from '@gitbutler/shared/context'; @@ -108,10 +108,11 @@ try { if (!project) return; for (let file of item.files) { - const absPath = await join(project.vscodePath, file.path); - openExternalUrl( - `${$userSettings.defaultCodeEditor.schemeIdentifer}://file${absPath}` - ); + const path = getEditorUri({ + schemeId: $userSettings.defaultCodeEditor.schemeIdentifer, + path: [project.vscodePath, file.path] + }); + openExternalUrl(path); } contextMenu.close(); } catch { diff --git a/apps/desktop/src/lib/hunk/HunkContextMenu.svelte b/apps/desktop/src/lib/hunk/HunkContextMenu.svelte index d635454cb..9b1c69e47 100644 --- a/apps/desktop/src/lib/hunk/HunkContextMenu.svelte +++ b/apps/desktop/src/lib/hunk/HunkContextMenu.svelte @@ -3,7 +3,7 @@ import ContextMenuItem from '$lib/components/contextmenu/ContextMenuItem.svelte'; import ContextMenuSection from '$lib/components/contextmenu/ContextMenuSection.svelte'; import { SETTINGS, type Settings } from '$lib/settings/userSettings'; - import { openExternalUrl } from '$lib/utils/url'; + import { getEditorUri, openExternalUrl } from '$lib/utils/url'; import { BranchController } from '$lib/vbranches/branchController'; import { getContextStoreBySymbol } from '@gitbutler/shared/context'; import { getContext } from '@gitbutler/shared/context'; @@ -49,9 +49,12 @@ label="Open in {$userSettings.defaultCodeEditor.displayName}" onclick={() => { if (projectPath) { - openExternalUrl( - `${$userSettings.defaultCodeEditor.schemeIdentifer}://file${projectPath}/${filePath}:${item.lineNumber}` - ); + const path = getEditorUri({ + schemeId: $userSettings.defaultCodeEditor.schemeIdentifer, + path: [projectPath, filePath], + line: item.lineNumber + }); + openExternalUrl(path); } contextMenu?.close(); }} diff --git a/apps/desktop/src/lib/utils/url.test.ts b/apps/desktop/src/lib/utils/url.test.ts index 47c4ddb9e..a12fb10a0 100644 --- a/apps/desktop/src/lib/utils/url.test.ts +++ b/apps/desktop/src/lib/utils/url.test.ts @@ -1,4 +1,4 @@ -import { remoteUrlIsHttp, convertRemoteToWebUrl } from '$lib/utils/url'; +import { remoteUrlIsHttp, convertRemoteToWebUrl, getEditorUri } from '$lib/utils/url'; import { describe, expect, test } from 'vitest'; describe.concurrent('cleanUrl', () => { @@ -42,3 +42,71 @@ describe.concurrent('cleanUrl', () => { expect(remoteUrlIsHttp(remoteUrl)).toBe(false); }); }); + +describe.concurrent('getEditorUri', () => { + test('it should handle editor path with no search params', () => { + expect(getEditorUri({ schemeId: 'vscode', path: ['/path', 'to', 'file'] })).toEqual( + 'vscode://file/path/to/file' + ); + }); + + test('it should handle editor path with search params', () => { + expect( + getEditorUri({ + schemeId: 'vscode', + path: ['/path', 'to', 'file'], + searchParams: { something: 'cool' } + }) + ).toEqual('vscode://file/path/to/file?something=cool'); + }); + + test('it should handle editor path with search params with special characters', () => { + expect( + getEditorUri({ + schemeId: 'vscode', + path: ['/path', 'to', 'file'], + searchParams: { + search: 'hello world', + what: 'bye-&*%*\\ded-yeah' + } + }) + ).toEqual('vscode://file/path/to/file?search=hello+world&what=bye-%26*%25*%5Cded-yeah'); + }); + + test('it should handle editor path with search params with line number', () => { + expect( + getEditorUri({ + schemeId: 'vscode', + path: ['/path', 'to', 'file'], + line: 10 + }) + ).toEqual('vscode://file/path/to/file:10'); + }); + + test('it should handle editor path with search params with line and column number', () => { + expect( + getEditorUri({ + schemeId: 'vscode', + path: ['/path', 'to', 'file'], + searchParams: { + another: 'thing' + }, + line: 10, + column: 20 + }) + ).toEqual('vscode://file/path/to/file:10:20?another=thing'); + }); + + test('it should ignore the column if there is no line number', () => { + expect( + getEditorUri({ + schemeId: 'vscode', + path: ['/path', 'to', 'file'], + searchParams: { + another: 'thing' + }, + column: 20 + }) + ).toEqual('vscode://file/path/to/file?another=thing'); + }); +}); diff --git a/apps/desktop/src/lib/utils/url.ts b/apps/desktop/src/lib/utils/url.ts index db05ad0d3..fe0af5daf 100644 --- a/apps/desktop/src/lib/utils/url.ts +++ b/apps/desktop/src/lib/utils/url.ts @@ -3,6 +3,8 @@ import { showToast } from '$lib/notifications/toasts'; import GitUrlParse from 'git-url-parse'; import { posthog } from 'posthog-js'; +const SEPARATOR = '/'; + export async function openExternalUrl(href: string) { try { await invoke('open_url', { url: href }); @@ -39,3 +41,30 @@ export function remoteUrlIsHttp(url: string): boolean { return httpProtocols.includes(gitRemote.protocol); } + +export interface EditorUriParams { + schemeId: string; + path: string[]; + searchParams?: Record; + line?: number; + column?: number; +} + +export function getEditorUri(params: EditorUriParams): string { + const searchParamsString = new URLSearchParams(params.searchParams).toString(); + // Separator is always a forward slash for editor paths, even on Windows + const pathString = params.path.join(SEPARATOR); + + let positionSuffix = ''; + if (params.line !== undefined) { + positionSuffix += `:${params.line}`; + // Column is only valid if line is present + if (params.column !== undefined) { + positionSuffix += `:${params.column}`; + } + } + + const searchSuffix = searchParamsString ? `?${searchParamsString}` : ''; + + return `${params.schemeId}://file${pathString}${positionSuffix}${searchSuffix}`; +}