GH-1905: Implemented the file download functionality. #2114

kittaakos · 2018-06-14T15:59:59Z

Closes #1905.

Signed-off-by: Akos Kitta kittaakos@gmail.com

kittaakos · 2018-06-15T11:34:13Z

It is ready for the review.

svenefftinge

We could move most of this to filesystem. The context menu registration could be moved to navigator.

svenefftinge · 2018-06-18T07:48:18Z

packages/core/src/common/objects.ts

+/**
+ * `true` if the argument is an empty object. Otherwise, `false`.
+ */
+export function isEmpty(arg: Object): boolean {


I think we also need to check the constructor : arg.constructor === Object as in theory there can be other objects without own properties.

svenefftinge · 2018-06-18T08:07:23Z

packages/file-download/src/node/file-download-endpoint.ts

+@injectable()
+export class FileDownloadEndpoint implements BackendApplicationContribution {
+
+    protected static PATH = '/file-download';


Should be just files.

svenefftinge · 2018-06-18T08:15:00Z

packages/file-download/src/browser/file-download-command-contribution.ts

+        if (uris.length === 1) {
+            return true;
+        }
+        // Can download multiple files iff they are from the same container folder.


Why restrict it like that?

I am OK removing the restriction if you could help me define a clear way how the files should be zipped up in a folder when selecting multiple files "randomly" from a workspace.

We could preserve the structure and use the common parent of all files as the container.

I had the same idea, then I thought; we have to overcomplicate the backend logic for a feature which we won't use that frequently anyway, but let's do.

kittaakos · 2018-06-18T08:25:18Z

We could move most of this to filesystem. The context menu registration could be moved to navigator.

I did not want to. I think it is nice to have it in its extension. It is always easier to customize and put together an application from tiny extensions than registering my own sub-classes, override methods here and there and then rebind then for instance, when I want to disable/change the default functionality.

But I could move it to the WS and FS.

svenefftinge · 2018-06-18T08:37:47Z

I would prefer it. Each extension adds a lot of infrastructural overhead (and degrades build performance) so it needs to be balanced when we create a new extension. I think in this case it tends towards having it in file-system as it is an endpoint exposing it.

kittaakos · 2018-06-18T08:40:52Z

Each extension adds a lot of infrastructural overhead (and degrades build performance)

It is unfortunate, we have to define extensions and adapt the code to this, but I see your point.

svenefftinge · 2018-06-18T08:41:35Z

Also, could we use tar instead of zip?
Zipping takes too long and blocks the node process. Also Users don't get any kind of progress feedback. Using tar instead makes that part fast and moves the work to the download, which has already a nice progress :).

kittaakos · 2018-06-18T08:46:50Z

Also, could we use tar instead of zip?

What can I do with a tar as a Windows user? I am just asking... I am not aware of any built-in tools that unpack the tar on Windows. Is it a good idea to force users to use 7Zip?

svenefftinge · 2018-06-19T06:45:15Z

@kittaakos did you see the build failures?

kittaakos · 2018-06-19T06:46:37Z

Thanks for the note: no. I am still in the middle of the changes; the commits are not yet squashed... I will ping you here when it is ready for the review.

kittaakos · 2018-06-19T15:36:47Z

@svenefftinge, I have made a few changes regarding the multi-file download feature; I would consider this PR ready for review.

svenefftinge · 2018-06-20T04:19:30Z

Downloading a directory (e.g. packages in theia repo) is still taking far too long to go without any kind of user feedback. We should at least add an item to the statusbar : preparing download ... with some spinning icon or so until the response comes in.

kittaakos · 2018-06-20T04:38:33Z

We should at least add an item to the statusbar : preparing download ... with some spinning icon or so until the response comes in.

OK.

kittaakos · 2018-06-20T06:57:49Z

I did some measurements, and the bottleneck is the archiving part: this takes 29430 ms when downloading the node_modules folder (573.2 MB) from Theia. I will try to pipe the archiving stream into the response. Hopefully, it makes the user experience smoother, if no, I update the status bar to give some user feedback.

kittaakos · 2018-06-20T08:13:29Z

I have managed to pipe the tar stream into the response but the download progress indicator did not show up in the browser even with downloadjs. I go with the status bar approach.

kittaakos · 2018-06-20T11:49:56Z

We should at least add an item to the statusbar : preparing download ... with some spinning icon or so until the response comes in.

This is done.

kittaakos · 2018-06-20T16:08:23Z

I tried to fine-tune the status bar's Preparing download... visibility. It is still not perfect; see the attached screen-casts below. I have already moved the status bar updating logic to the very last possible position where I still do "have control" over the download.

This could be the root of the defect (source: whatwg/html#954 (comment)):

[...]The problem is that browsers tend to start the download asynchronously after the link has been clicked. This is especially browsers which show the user a "do you want to open or download this file?" dialog, where the download might start long after the link was clicked.[...]

Downloading packages:

Downloading node_modules (cut):

Downloading theia (cut):

Closes #1905. Signed-off-by: Akos Kitta <kittaakos@gmail.com>

svenefftinge

Thank you!

davide-sergi · 2021-06-18T13:11:44Z

Is there a flow diagram somewhere in order to understand how download a file/folder programmatically?

kittaakos · 2021-06-18T16:56:30Z

flow diagram

No, there isn't. At least, I am not aware of it.

how download a file/folder programmatically?

Check this thread. It might be outdated, though. If it does not work now, please open a follow-up issue, or start a discussion, to attract more attention for your problem. This is a closed PR.

I hope this helps.

kittaakos force-pushed the GH-1905 branch from 9478bb7 to f03dde3 Compare June 15, 2018 11:33

kittaakos changed the title ~~[WIP] GH-1905: Implemented the single file download functionality.~~ GH-1905: Implemented the file download functionality. Jun 15, 2018

kittaakos requested a review from svenefftinge June 15, 2018 11:34

kittaakos force-pushed the GH-1905 branch 2 times, most recently from 4e1c3f1 to d567576 Compare June 15, 2018 13:44

svenefftinge reviewed Jun 18, 2018

View reviewed changes

kittaakos force-pushed the GH-1905 branch from 76bf1b2 to 98488de Compare June 19, 2018 15:30

kittaakos force-pushed the GH-1905 branch from 98488de to 1f1a7fd Compare June 20, 2018 10:14

GH-1905: Implemented the file download functionality.

871bcc7

Closes #1905. Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos force-pushed the GH-1905 branch from 1f1a7fd to 871bcc7 Compare June 20, 2018 16:10

svenefftinge approved these changes Jun 22, 2018

View reviewed changes

kittaakos merged commit 4dfc327 into master Jun 22, 2018

kittaakos deleted the GH-1905 branch June 22, 2018 08:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-1905: Implemented the file download functionality. #2114

GH-1905: Implemented the file download functionality. #2114

kittaakos commented Jun 14, 2018 •

edited

Loading

kittaakos commented Jun 15, 2018

svenefftinge left a comment

svenefftinge Jun 18, 2018

kittaakos Jun 18, 2018

svenefftinge Jun 18, 2018

kittaakos Jun 18, 2018

svenefftinge Jun 18, 2018

kittaakos Jun 18, 2018

svenefftinge Jun 18, 2018

kittaakos Jun 18, 2018

kittaakos commented Jun 18, 2018

svenefftinge commented Jun 18, 2018

kittaakos commented Jun 18, 2018

svenefftinge commented Jun 18, 2018

kittaakos commented Jun 18, 2018

svenefftinge commented Jun 19, 2018

kittaakos commented Jun 19, 2018

kittaakos commented Jun 19, 2018

svenefftinge commented Jun 20, 2018

kittaakos commented Jun 20, 2018

kittaakos commented Jun 20, 2018

kittaakos commented Jun 20, 2018

kittaakos commented Jun 20, 2018

kittaakos commented Jun 20, 2018

svenefftinge left a comment

davide-sergi commented Jun 18, 2021

kittaakos commented Jun 18, 2021

GH-1905: Implemented the file download functionality. #2114

GH-1905: Implemented the file download functionality. #2114

Conversation

kittaakos commented Jun 14, 2018 • edited Loading

kittaakos commented Jun 15, 2018

svenefftinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kittaakos commented Jun 18, 2018

svenefftinge commented Jun 18, 2018

kittaakos commented Jun 18, 2018

svenefftinge commented Jun 18, 2018

kittaakos commented Jun 18, 2018

svenefftinge commented Jun 19, 2018

kittaakos commented Jun 19, 2018

kittaakos commented Jun 19, 2018

svenefftinge commented Jun 20, 2018

kittaakos commented Jun 20, 2018

kittaakos commented Jun 20, 2018

kittaakos commented Jun 20, 2018

kittaakos commented Jun 20, 2018

kittaakos commented Jun 20, 2018

svenefftinge left a comment

Choose a reason for hiding this comment

davide-sergi commented Jun 18, 2021

kittaakos commented Jun 18, 2021

kittaakos commented Jun 14, 2018 •

edited

Loading