Align upload & download progress between desktop and web

[bpasero]

Testing #111250

I think the download progress for web is more detailed compared to what we have now for desktop and it would be great to align. Though I am not sure if this possibly requires some infrastructure on the file service layer.

Update from #146226: The same applies to upload progress which is much nicer in web.

We have a few places where we show progress for uploading, downloading and importing:

vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts

Line 89 in 32d40cf

const uploadPromise = this.progressService.withProgress(

vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts

Line 409 in 32d40cf

const importPromise = this.progressService.withProgress(

vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts

Line 590 in 32d40cf

const downloadPromise = this.progressService.withProgress(

In all three cases we can use the IProgressStep to update the message and overall progress counts.

We may not get all the information we need such as knowing the overall size, but if we do we should make sure that the label for the progress is as accurate as possible.

This sounds like a fun area for a contribution where someone can geek out on providing the best progress info possible. As such, opening for help wanted.

[isidorn]

Web uses a custom solution which only works for that use case, I want a general approach for all file operations.
I would not change this for now -> backlog.

Custom solution code pointer

vscode/src/vs/workbench/contrib/files/browser/views/explorerViewer.ts

Line 1019 in 7536644

const dropPromise = this.progressService.withProgress({

General solution code pointer

vscode/src/vs/workbench/contrib/files/browser/explorerService.ts

Line 98 in 7536644

const promise = this.progressService.withProgress({

[isidorn]

To make the general solution as powerful as the custom one for upload we would need some infrastrucutre on the file service layer as you mentioned.
Before we have that assigning this to On-Deck

[sbatten]

I was testing one of the e2e scenarios and notice that in web, the status bar indicator is easily missed. I could not tell that it was still downloading until I opened the locally and saw it was still populating.

[DanTaranis]

@lramos15 how is this going?

[lramos15]

@lramos15 how is this going?

There has been no progress here, PRs are welcome

[kkneze01]

We are missing this feature in VSCode

[v4r4rth]

Adding to the noise - would love this feature..

[JalinWang]

#83565 (comment) The web version is very nice

[JalinWang]

I am attempting to address this issue and leaving some notes to ensure at least a little contribution, just in case I decide to give up at some point. 😥
Feel free to point out my mistakes.

[JalinWang]

The uploading operation starts in FileDragAndDrop.drop() function in src/vs/workbench/contrib/files/browser/views/explorerViewer.ts (as well as uploading menu in browser

vscode/src/vs/workbench/contrib/files/browser/fileActions.ts

Line 1078 in f8c7fec

const uploadFileHandler = async (accessor: ServicesAccessor) => {

)

vscode/src/vs/workbench/contrib/files/browser/views/explorerViewer.ts

Lines 1284 to 1301 in 14d0156

	// External file DND (Import/Upload file)
	if (data instanceof NativeDragAndDropData) {
	// Use local file import when supported
	if (!isWeb || (isTemporaryWorkspace(this.contextService.getWorkspace()) && WebFileSystemAccess.supported(mainWindow))) {
	const fileImport = this.instantiationService.createInstance(ExternalFileImport);
	await fileImport.import(resolvedTarget, originalEvent, mainWindow);
	}
	// Otherwise fallback to browser based file upload
	else {
	const browserUpload = this.instantiationService.createInstance(BrowserFileUpload);
	await browserUpload.upload(target, originalEvent);
	}
	}
	// In-Explorer DND (Move/Copy file)
	else {
	await this.handleExplorerDrop(data as ElementsDragAndDropData<ExplorerItem, ExplorerItem[]>, resolvedTarget, originalEvent);
	}

Codes here choose different behaviors for different platforms.
Currently, BrowserFileUpload for web already has a nice progress bar.

Thus, we should focus on ExternalFileImport class in src/vs/workbench/contrib/files/browser/fileImportExport.ts.

For downloading, the platform switching logic is located in FileDownload.doDownload() in src/vs/workbench/contrib/files/browser/fileImportExport.ts and we should improve FileDownload.doDownloadNative() function.

vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts

Lines 633 to 642 in 14d0156

	// Web: use DOM APIs to download files with optional support
	// for folders and large files
	if (isWeb) {
	await this.doDownloadBrowser(source.resource, progress, cts);
	}
	// Native: use working copy file service to get at the contents
	else {
	await this.doDownloadNative(source, progress, cts);
	}

[JalinWang]

Take downloading as an example, there are several progress indicators:

1. in file tree view:
   

   vscode/src/vs/workbench/contrib/files/common/files.ts

   Lines 31 to 34 in 14d0156

   	/**
   	* Explorer file view id.
   	*/
   	export const VIEW_ID = 'workbench.explorer.fileView';

   
   

   vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts

   Lines 621 to 622 in 14d0156

   	// Also indicate progress in the files view
   	this.progressService.withProgress({ location: VIEW_ID, delay: 500 }, () => downloadPromise);

2. notification of the coping state for the whole operation:
   

   vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts

   Lines 609 to 620 in 14d0156

   	// Indicate progress globally
   	const downloadPromise = this.progressService.withProgress(
   	{
   	location: ProgressLocation.Window,
   	delay: 800,
   	cancellable: isWeb,
   	title: localize('downloadingFiles', "Downloading")
   	},
   	async progress => this.doDownload(source, progress, cts),
   	() => cts.dispose(true)
   	);

3. notification of the coping state for current file:
   

   vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts

   Lines 837 to 845 in 14d0156

   	// Remember as last used download folder
   	this.storageService.store(FileDownload.LAST_USED_DOWNLOAD_PATH_STORAGE_KEY, dirname(destination).fsPath, StorageScope.APPLICATION, StorageTarget.MACHINE);
   	// Perform download
   	await this.explorerService.applyBulkEdit([new ResourceFileEdit(explorerItem.resource, destination, { overwrite: true, copy: true })], {
   	undoLabel: localize('downloadBulkEdit', "Download {0}", explorerItem.name),
   	progressLabel: localize('downloadingBulkEdit', "Downloading {0}", explorerItem.name),
   	progressLocation: ProgressLocation.Window
   	});

4. indicator in file view for individual file:
   

   vscode/src/vs/workbench/contrib/files/browser/explorerService.ts

   Line 202 in 14d0156

   await this.progressService.withProgress({ location: ProgressLocation.Explorer, delay: 500 }, () => promise);

[JalinWang]

The operations are actually performed in bulkEditService by delegation from https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/files/browser/explorerService.ts#L185-L204

vscode/src/vs/workbench/contrib/bulkEdit/browser/bulkEditService.ts

Line 245 in 14d0156

const resources = await bulkEdit.perform();

vscode/src/vs/workbench/contrib/bulkEdit/browser/bulkEditService.ts

Line 91 in 14d0156

async perform(): Promise<readonly URI[]> {

[nom]

+1 this would be a great feature

[JalinWang]

BTW, the uploading operation is not cancelable for remote dev although there are cancel buttons :(

Downloading instead doesn't show the cancel option.

updated: root cause
https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/files/browser/explorerService.ts#L190

[JalinWang]

As isidorn & bpasero say, we'd better

1. create an abstraction/interface to replace the bulkEdit usage for uploading/downloading as well as unify the web&native version. (if I got it correctly);
2. make the fs layer report copied/transfered bytes.

[JalinWang]

The copy op happens here:

vscode/src/vs/platform/files/common/fileService.ts

Lines 789 to 809 in 14d0156

	// copy source => target
	if (mode === 'copy') {
	// same provider with fast copy: leverage copy() functionality
	if (sourceProvider === targetProvider && hasFileFolderCopyCapability(sourceProvider)) {
	await sourceProvider.copy(source, target, { overwrite });
	}
	// when copying via buffer/unbuffered, we have to manually
	// traverse the source if it is a folder and not a file
	else {
	const sourceFile = await this.resolve(source);
	if (sourceFile.isDirectory) {
	await this.doCopyFolder(sourceProvider, sourceFile, targetProvider, target);
	} else {
	await this.doCopyFile(sourceProvider, source, targetProvider, target);
	}
	}
	return mode;
	}

For remote scenarios, the doCopyFile does the actual work.

vscode/src/vs/platform/files/common/fileService.ts

Lines 831 to 852 in 14d0156

	private async doCopyFile(sourceProvider: IFileSystemProvider, source: URI, targetProvider: IFileSystemProvider, target: URI): Promise<void> {
	// copy: source (buffered) => target (buffered)
	if (hasOpenReadWriteCloseCapability(sourceProvider) && hasOpenReadWriteCloseCapability(targetProvider)) {
	return this.doPipeBuffered(sourceProvider, source, targetProvider, target);
	}
	// copy: source (buffered) => target (unbuffered)
	if (hasOpenReadWriteCloseCapability(sourceProvider) && hasReadWriteCapability(targetProvider)) {
	return this.doPipeBufferedToUnbuffered(sourceProvider, source, targetProvider, target);
	}
	// copy: source (unbuffered) => target (buffered)
	if (hasReadWriteCapability(sourceProvider) && hasOpenReadWriteCloseCapability(targetProvider)) {
	return this.doPipeUnbufferedToBuffered(sourceProvider, source, targetProvider, target);
	}
	// copy: source (unbuffered) => target (unbuffered)
	if (hasReadWriteCapability(sourceProvider) && hasReadWriteCapability(targetProvider)) {
	return this.doPipeUnbuffered(sourceProvider, source, targetProvider, target);
	}
	}

One change of infra may do something with doPipe* API series here ( to report download/uploading progress)