Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FTP polish #34704

Closed
4 of 6 tasks
isidorn opened this issue Sep 20, 2017 · 7 comments
Closed
4 of 6 tasks

FTP polish #34704

isidorn opened this issue Sep 20, 2017 · 7 comments
Assignees
Labels
file-explorer Explorer widget issues plan-item VS Code - planned item for upcoming remote Remote system operations issues

Comments

@isidorn
Copy link
Contributor

isidorn commented Sep 20, 2017

  • verify file matching did not get broken on case insensitive file systems
  • Try to lazily add the FTP folder once ready, since currently the whole explorer is loading on startup
  • Fix broken copy ftp folder action -> remote - support copy folder action #41547
  • Think about UI to add a new ftp root -> that's extension buisness

from @bpasero

  • fileActions seems to miss proper handling of the resource, e.g. not all resources are saveable
  • textModelResolverService should this also use the supportsResource check?
  • textFileService is probably OK because non-saveable resources will not return as dirty
@isidorn isidorn added file-explorer Explorer widget issues remote Remote system operations issues labels Sep 20, 2017
@isidorn isidorn added this to the September 2017 milestone Sep 20, 2017
@jrieken
Copy link
Member

jrieken commented Sep 20, 2017

editorService will always evaluate to true even if supportResource returns false

@bpasero Unsure, I’d say && binds stronger than || but I might be missing something

textModelResolverService should this also use the supportsResource check?

I kind of does, just the inverse by checking if the scheme is supported from the other side and defaults to the checking with the file service

@bpasero
Copy link
Member

bpasero commented Sep 20, 2017

Unsure, I’d say && binds stronger than || but I might be missing something

True, my mental compiler does not seem to follow the spec...

@jrieken
Copy link
Member

jrieken commented Sep 22, 2017

fileActions seems to miss proper handling of the resource, e.g. not all resources are saveable

Shouldn't this be decided by the service? It will throw a ENOPRO error when being called with an unknown scheme and things like the save action should simply handle such errors. I believe that makes things easier as it must handle save failure anyways

@bpasero
Copy link
Member

bpasero commented Sep 22, 2017

@jrieken yeah that could work. the normal save action seems to just do nothing silently when the resource is not supported (I tried File > Save of a staged resource with git scheme). However, doing "Save As..." results in this:

image

jrieken added a commit that referenced this issue Sep 22, 2017
@jrieken
Copy link
Member

jrieken commented Sep 22, 2017

Yeah, so the service works as expected and with my latest commit it stops printing the toString-implementation of URI but actually invokes it.

Maybe we are missing well defines error code and a standard file error? I did start to define some for the provider apis: https://github.com/Microsoft/vscode/blob/master/src/vs/vscode.proposed.d.ts#L39 and believe a super set of those should be well known to everyone dealing with the file service

@bpasero
Copy link
Member

bpasero commented Sep 22, 2017

Yeah I am already using similar things for the file service from FileOperationResult. They are more high level than the basic node.js fs ones and I deal with those in most places where I call the file service and if not, they produce a human readable error message at least.

@jrieken jrieken modified the milestones: September 2017, October 2017 Sep 22, 2017
@jrieken jrieken removed this from the October 2017 milestone Oct 27, 2017
@jrieken jrieken added the plan-item VS Code - planned item for upcoming label Nov 13, 2017
@jrieken
Copy link
Member

jrieken commented Jan 12, 2018

Closing as we have shelled out most items into separate issues or fixed them.

@jrieken jrieken closed this as completed Jan 12, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Feb 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
file-explorer Explorer widget issues plan-item VS Code - planned item for upcoming remote Remote system operations issues
Projects
None yet
Development

No branches or pull requests

3 participants