Repository navigation
[Feat]: Add option to disable "Show Local" while saving #5118
Description
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.
Activity
[-][Feat]: Add option for disabling "Show Local" while saving[/-][+][Feat]: Add option to disable "Show Local" while saving[/+]jsjoeio commentedon Apr 19, 2022
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 commentedon Apr 20, 2022
Yes, it opens an explorer/finder popup asking where to save the file locally. After that the file gets downloaded.
This button
Show Localalso appears when you are opening a file viaFile -> 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 commentedon Nov 23, 2022
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 commentedon Sep 25, 2023
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.Filevalue ends up inavailableFileSystemslist 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 commentedon Sep 28, 2023
In code-server that will always be an
IBrowserWorkbenchEnvironmentServiceso there should be no problem. In desktop VS Code it would be anINativeWorkbenchEnvironmentService. 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 commentedon Nov 13, 2023
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).