feature: replace electron File.path with electron webUtils (#213031)

* add getPathForFIle function

* use electron webutils

* use webutils

* use getPathForFile

* add test

* fix import

* add web utils service

* register electron webutils service

* register contribution

* import contribution

* register web contribution

* feature: use webUtils service to not directly depend on electron global

* use webutils service for terminal

* update imports

* tsc

* update imports

* add servicebrand

* tsc

* tsc

* try different approach

* add class and subclass

* register contribution with subclass

* rename function

* remove web utils service

* register getPathForFile function

* fix import

* remove webutils service

* remove import

* remove import

* fix imports

* fix import

* fix overwriting view

* register method

* fix import

* register contribution

* don't overwrite electron service with browser service

* remove log

* remove log

* tsc

* Discard changes to src/vs/workbench/contrib/files/browser/fileActions.contribution.ts

* Discard changes to src/vs/workbench/contrib/files/browser/fileActions.ts

* Discard changes to src/vs/workbench/contrib/files/electron-sandbox/fileActions.contribution.ts

* Discard changes to src/vs/workbench/contrib/terminal/browser/terminal.contribution.ts

* Discard changes to src/vs/workbench/contrib/terminal/browser/terminal.ts

* Discard changes to src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

* Discard changes to src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts

* Discard changes to src/vs/workbench/contrib/terminal/browser/terminalTabbedView.ts

* Discard changes to src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts

* Discard changes to src/vs/workbench/contrib/terminal/browser/terminalView.ts

* Discard changes to src/vs/workbench/contrib/terminal/electron-sandbox/terminal.contribution.ts

* Discard changes to src/vs/workbench/contrib/terminal/electron-sandbox/terminalInstanceService.ts

* Discard changes to src/vs/workbench/contrib/terminal/electron-sandbox/terminalView.ts

* Discard changes to src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts

* add function to interface

* add function

* use host service

* use host service in terminal tabs list

* use host service in terminal instance

* remove unused code

* fix imports

* replace remaining usages of file path

* tsc

* add semicolon

* cleanup

---------

Co-authored-by: Benjamin Pasero <benjamin.pasero@microsoft.com>
This commit is contained in:
Simon Siefke 2024-06-02 20:09:17 +02:00 committed by GitHub
parent 60d7343892
commit acfe0e20ce
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 89 additions and 16 deletions

View file

@ -172,3 +172,19 @@ export interface AuthInfo {
port: number;
realm: string;
}
export interface WebUtils {
// Docs: https://electronjs.org/docs/api/web-utils
/**
* The file system path that this `File` object points to. In the case where the
* object passed in is not a `File` object an exception is thrown. In the case
* where the File object passed in was constructed in JS and is not backed by a
* file on disk an empty string is returned.
*
* This method superceded the previous augmentation to the `File` object with the
* `path` property. An example is included below.
*/
getPathForFile(file: File): string;
}

View file

@ -5,7 +5,7 @@
import { INodeProcess, IProcessEnvironment } from 'vs/base/common/platform';
import { ISandboxConfiguration } from 'vs/base/parts/sandbox/common/sandboxTypes';
import { IpcRenderer, ProcessMemoryInfo, WebFrame } from 'vs/base/parts/sandbox/electron-sandbox/electronTypes';
import { IpcRenderer, ProcessMemoryInfo, WebFrame, WebUtils } from 'vs/base/parts/sandbox/electron-sandbox/electronTypes';
/**
* In Electron renderers we cannot expose all of the `process` global of node.js
@ -121,6 +121,7 @@ export const ipcMessagePort: IpcMessagePort = vscodeGlobal.ipcMessagePort;
export const webFrame: WebFrame = vscodeGlobal.webFrame;
export const process: ISandboxNodeProcess = vscodeGlobal.process;
export const context: ISandboxContext = vscodeGlobal.context;
export const webUtils: WebUtils = vscodeGlobal.webUtils;
/**
* A set of globals that are available in all windows that either

View file

@ -7,7 +7,7 @@
(function () {
'use strict';
const { ipcRenderer, webFrame, contextBridge } = require('electron');
const { ipcRenderer, webFrame, contextBridge, webUtils } = require('electron');
//#region Utilities
@ -237,6 +237,19 @@
}
},
/**
* Support for subset of Electron's `webUtils` type.
*/
webUtils: {
/**
* @param {File} file
*/
getPathForFile(file) {
return webUtils.getPathForFile(file);
}
},
/**
* Support for a subset of access to node.js global `process`.
*

View file

@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import * as assert from 'assert';
import { context, ipcRenderer, process, webFrame } from 'vs/base/parts/sandbox/electron-sandbox/globals';
import { context, ipcRenderer, process, webFrame, webUtils } from 'vs/base/parts/sandbox/electron-sandbox/globals';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
suite('Sandbox', () => {
@ -13,6 +13,7 @@ suite('Sandbox', () => {
assert.ok(typeof ipcRenderer.send === 'function');
assert.ok(typeof webFrame.setZoomLevel === 'function');
assert.ok(typeof process.platform === 'string');
assert.ok(typeof webUtils.getPathForFile === 'function');
const config = await context.resolveConfiguration();
assert.ok(config);

View file

@ -1110,18 +1110,32 @@ export const pasteFileHandler = async (accessor: ServicesAccessor, fileList?: Fi
const configurationService = accessor.get(IConfigurationService);
const uriIdentityService = accessor.get(IUriIdentityService);
const dialogService = accessor.get(IDialogService);
const hostService = accessor.get(IHostService);
const context = explorerService.getContext(false);
const hasNativeFilesToPaste = fileList && fileList.length > 0;
const confirmPasteNative = hasNativeFilesToPaste && configurationService.getValue<boolean>('explorer.confirmPasteNative');
const toPaste = await getFilesToPaste(fileList, clipboardService);
const toPaste = await getFilesToPaste(fileList, clipboardService, hostService);
if (confirmPasteNative && toPaste.files.length >= 1) {
const message = toPaste.files.length > 1 ?
nls.localize('confirmMultiPasteNative', "Are you sure you want to paste the following {0} items?", toPaste.files.length) :
nls.localize('confirmPasteNative', "Are you sure you want to paste '{0}'?", basename(toPaste.type === 'paths' ? toPaste.files[0].fsPath : toPaste.files[0].name));
const detail = toPaste.files.length > 1 ? getFileNamesMessage(toPaste.files.map(item => toPaste.type === 'paths' ? item.path : (item as File).name)) : undefined;
const detail = toPaste.files.length > 1 ? getFileNamesMessage(toPaste.files.map(item => {
if (URI.isUri(item)) {
return item.fsPath;
}
if (toPaste.type === 'paths') {
const path = hostService.getPathForFile(item);
if (path) {
return path;
}
}
return item.name;
})) : undefined;
const confirmation = await dialogService.confirm({
message,
detail,
@ -1270,16 +1284,16 @@ type FilesToPaste =
| { type: 'paths'; files: URI[] }
| { type: 'data'; files: File[] };
async function getFilesToPaste(fileList: FileList | undefined, clipboardService: IClipboardService): Promise<FilesToPaste> {
async function getFilesToPaste(fileList: FileList | undefined, clipboardService: IClipboardService, hostService: IHostService): Promise<FilesToPaste> {
if (fileList && fileList.length > 0) {
// with a `fileList` we support natively pasting file from disk from clipboard
const resources = [...fileList].filter(file => !!file.path && isAbsolute(file.path)).map(file => URI.file(file.path));
const resources = [...fileList].map(file => hostService.getPathForFile(file)).filter(filePath => !!filePath && isAbsolute(filePath)).map((filePath) => URI.file(filePath!));
if (resources.length) {
return { type: 'paths', files: resources, };
}
// Support pasting files that we can't read from disk
return { type: 'data', files: [...fileList].filter(file => !file.path) };
return { type: 'data', files: [...fileList].filter(file => !hostService.getPathForFile(file)) };
} else {
// otherwise we fallback to reading resources from our clipboard service
return { type: 'paths', files: resources.distinctParents(await clipboardService.readResources(), resource => resource) };

View file

@ -88,6 +88,7 @@ import { AccessibilityCommandId } from 'vs/workbench/contrib/accessibility/commo
import { terminalStrings } from 'vs/workbench/contrib/terminal/common/terminalStrings';
import { shouldPasteTerminalText } from 'vs/workbench/contrib/terminal/common/terminalClipboard';
import { TerminalIconPicker } from 'vs/workbench/contrib/terminal/browser/terminalIconPicker';
import { IHostService } from 'vs/workbench/services/host/browser/host';
// HACK: This file should not depend on terminalContrib
// eslint-disable-next-line local/code-import-patterns
@ -2288,6 +2289,7 @@ class TerminalInstanceDragAndDropController extends Disposable implements dom.ID
private readonly _container: HTMLElement,
@IWorkbenchLayoutService private readonly _layoutService: IWorkbenchLayoutService,
@IViewDescriptorService private readonly _viewDescriptorService: IViewDescriptorService,
@IHostService private readonly _hostService: IHostService,
) {
super();
this._register(toDisposable(() => this._clearDropOverlay()));
@ -2372,9 +2374,9 @@ class TerminalInstanceDragAndDropController extends Disposable implements dom.ID
path = URI.file(JSON.parse(rawCodeFiles)[0]);
}
if (!path && e.dataTransfer.files.length > 0 && e.dataTransfer.files[0].path /* Electron only */) {
if (!path && e.dataTransfer.files.length > 0 && this._hostService.getPathForFile(e.dataTransfer.files[0])) {
// Check if the file was dragged from the filesystem
path = URI.file(e.dataTransfer.files[0].path);
path = URI.file(this._hostService.getPathForFile(e.dataTransfer.files[0])!);
}
if (!path) {

View file

@ -51,6 +51,7 @@ import { Schemas } from 'vs/base/common/network';
import { getColorForSeverity } from 'vs/workbench/contrib/terminal/browser/terminalStatusList';
import { TerminalContextActionRunner } from 'vs/workbench/contrib/terminal/browser/terminalContextMenu';
import type { IHoverAction } from 'vs/base/browser/ui/hover/hover';
import { IHostService } from 'vs/workbench/services/host/browser/host';
const $ = DOM.$;
@ -577,6 +578,7 @@ class TerminalTabsDragAndDrop extends Disposable implements IListDragAndDrop<ITe
constructor(
@ITerminalService private readonly _terminalService: ITerminalService,
@ITerminalGroupService private readonly _terminalGroupService: ITerminalGroupService,
@IHostService private readonly _hostService: IHostService,
) {
super();
this._primaryBackend = this._terminalService.getPrimaryBackend();
@ -733,9 +735,9 @@ class TerminalTabsDragAndDrop extends Disposable implements IListDragAndDrop<ITe
resource = URI.file(JSON.parse(rawCodeFiles)[0]);
}
if (!resource && e.dataTransfer.files.length > 0 && e.dataTransfer.files[0].path /* Electron only */) {
if (!resource && e.dataTransfer.files.length > 0 && this._hostService.getPathForFile(e.dataTransfer.files[0])) {
// Check if the file was dragged from the filesystem
resource = URI.file(e.dataTransfer.files[0].path);
resource = URI.file(this._hostService.getPathForFile(e.dataTransfer.files[0])!);
}
if (!resource) {

View file

@ -576,6 +576,14 @@ export class BrowserHostService extends Disposable implements IHostService {
}
//#endregion
//#region File
getPathForFile(): undefined {
return undefined; // unsupported in browser environments
}
//#endregion
}
registerSingleton(IHostService, BrowserHostService, InstantiationType.Delayed);

View file

@ -19,7 +19,6 @@ export interface IHostService {
readonly _serviceBrand: undefined;
//#region Focus
/**
@ -56,7 +55,6 @@ export interface IHostService {
//#endregion
//#region Window
/**
@ -123,4 +121,10 @@ export interface IHostService {
withExpectedShutdown<T>(expectedShutdownTask: () => Promise<T>): Promise<T>;
//#endregion
//#region File
getPathForFile(file: File): string | undefined;
//#endregion
}

View file

@ -17,6 +17,7 @@ import { IMainProcessService } from 'vs/platform/ipc/common/mainProcessService';
import { disposableWindowInterval, getActiveDocument, getWindowId, getWindowsCount, hasWindow, onDidRegisterWindow } from 'vs/base/browser/dom';
import { memoize } from 'vs/base/common/decorators';
import { isAuxiliaryWindow } from 'vs/base/browser/window';
import { webUtils } from 'vs/base/parts/sandbox/electron-sandbox/globals';
class WorkbenchNativeHostService extends NativeHostService {
@ -66,7 +67,6 @@ class WorkbenchHostService extends Disposable implements IHostService {
//#endregion
//#region Window
@memoize
@ -160,7 +160,6 @@ class WorkbenchHostService extends Disposable implements IHostService {
//#endregion
//#region Lifecycle
focus(targetWindow: Window, options?: { force: boolean }): Promise<void> {
@ -187,6 +186,15 @@ class WorkbenchHostService extends Disposable implements IHostService {
}
//#endregion
//#region File
getPathForFile(file: File): string {
return webUtils.getPathForFile(file);
}
//#endregion
}
registerSingleton(IHostService, WorkbenchHostService, InstantiationType.Delayed);

View file

@ -1545,6 +1545,10 @@ export class TestHostService implements IHostService {
readonly colorScheme = ColorScheme.DARK;
onDidChangeColorScheme = Event.None;
getPathForFile(file: File): string | undefined {
return undefined;
}
}
export class TestFilesConfigurationService extends FilesConfigurationService {