[Feat]: Add option to disable "Show Local" while saving

[avsthiago-kbc]

What is your suggestion?

Reuse the option --disable-file-downloads recently implemented on #5055 to remove the Show Local button when using the Save As option. This option allows the users to save the current file on their local machine.

Example:

Why do you want this feature?

One could argue that having the option to save the file locally makes the download as accessible as having the Download button when right clicking the file name. As mentioned in the comments on the issue #1386, some industries have concerns about downloading files from the IDE.

Are there any workarounds to get this functionality today?

Not that I'm aware of.

How this feature could be implemented?

We could extend the code implemented on #5055. The value of the option --disable-file-downloads could be used within a condition that will either add the "Show Local" button or not. As I understand, the button is added on simpleFileDialog.ts L279. Disclaimer, I'm not familiar with the code base, neither TypeScript, so there might be more places we will need patch.

Are you interested in submitting a PR for this?

Yes, but I might need some guidance setting up the environment.

[jsjoeio]

This option allows the users to save the current file on their local machine.

Interesting! So this gives you access to the files on the local machine? I don't think we realized that. This is an interesting feature request though.

[avsthiago-kbc]

Yes, it opens an explorer/finder popup asking where to save the file locally. After that the file gets downloaded.

This button Show Local also appears when you are opening a file via File -> Open File.... If we decide to disable the button for the download in the simpleFileDialog.ts it might also impact the upload. We have to be aware of this side effect.

[paulchill]

yeah it's a bit pointless removing file download but having this option, i would say --disable-file-downloads and --disable-show-local option should go hand in hand - also there is security issues with uploads too the way there are issues / code / ip leaks with downloads..

[jeffmax]

I've traced through the code and I believe the place to address this is within this function:

addFileSchemaIfNeeded function in fileDialogService.ts

Specifically, my understanding is that if the Schema.File value ends up in availableFileSystems list then the "Show Local" button will be made available. I could be misunderstanding the intent behind these schemas, but in my testing, this is the behavior.

To @avsthiago-kbc comment above, this function is used in several places and would require passing a context variable to clarify whether the context is `save' or 'open'.

I have pulled together some working code for this, but one of my biggest confusion points is around the appropriate way to propagate the disable-file-downloads option down to this location in the code. Initially I was trying to follow the way this was done in the patch that introduced the flag, but it was making use of ContextKeys and using those in if-then statements didn't seem correct. I ended up using the environmentService variable that was already on abstractFileDialogService, but I had to change it from IWorkbenchEnvironmentService to IBrowserWorkbenchEnvironmentService. I noticed that this same change was made in src/vs/workbench/browser/contextkeys.ts in the PR to add the disable-downloads flag but I don't fully understand the ramifications.

[code-asher]

In code-server that will always be an IBrowserWorkbenchEnvironmentService so there should be no problem. In desktop VS Code it would be an INativeWorkbenchEnvironmentService. So if we wanted to upstream this change eventually we would have to change the way we do it, but for now I think assuming it is the browser service is fine.

[jeffmax]

I just created a PR for this. I initially took the approach that I discussed in my above comment to control the contents of availableFileSystems, however I decided this was more confusing and modified code that a lot of actions pass through. Ultimately I modified the function that @avsthiago-kbc first mentioned as the intent was more clear and the appropriate context was available as to not disable "Show Local" for opening files (as a concern was raised above).