-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Enable opening of remote(http|https) files in Brackets #14153
Conversation
How are the users supposed to trigger opening of a remote file ? Is there any UI associated with it ? |
@vickramdhawal It can be done by file open command. CommandManager.execute(Commands.FILE_OPEN, {
fullPath: "<remote-file-uri here>"
}); [update] |
"protocol can be any valid file protocol string like HTTP: | HTTPS: | FTP: | SFTP:" |
Also would be good if this could be hooked into some UI. maybe the quick open cmd+o hotkey. |
@abose This PR provides implementation only for Let me see if this can be hooked to quickopen where user can type/paste a remote path to open a file. |
Now that quick open works on http/s URL , normal file[file:///c/a.txt or c:\a.txt etc] paths cannot be opened with the same. Would be good if this behaved consistently for file path open as well. [swmitra] - @abose I guess it never worked earlier with file path even without this PR. My take on this - can be taken as a separate PR! I will still do some quick check... |
|
||
var SESSION_START_TIME = new Date(); | ||
|
||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: /**
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in latest commit 👍
HTTPS_PROTOCOL = "https:"; | ||
|
||
AppInit.htmlReady(function () { | ||
var protocolAdapter = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is an extension shouldn't the definition of ProtocolAdapter be provided by Brackets-Core for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vickramdhawal Sorry for the confusion. protocolAdapter
is just a variable to hold the custom imprint of the adapter definition to be passed as protocol adapters for HTTP:
and HTTPS:
. Registration happens by invoking this part -
// Register the custom object as HTTP and HTTPS protocol adapter
FileSystem.registerProtocolAdapter(HTTP_PROTOCOL, protocolAdapter);
FileSystem.registerProtocolAdapter(HTTPS_PROTOCOL, protocolAdapter);
I can't really define an interface (which if was possible, would have been part of core) and Duck Typing
would be a gimmick as type checking is not something what we are interested about here.
For better understanding I have added a JSDoc @typedef
/**
* Typical signature of a file protocol adapter.
* @typedef {Object} FileProtocol~Adapter
* @property {Number} priority - Indicates the priority.
* @property {Object} fileImpl - Handle for the custom file implementation prototype.
* @property {function} canRead - To check if this impl can read a file for a given path.
*/
/**
* FileSystem hook to register file protocol adapter
* @param {string} protocol ex: "https:"|"http:"|"ftp:"|"file:"
* @param {...FileProtocol~Adapter} adapter wrapper over file implementation
*/
LGTM. |
/** | ||
* Helpful toString for debugging and equality check purposes | ||
*/ | ||
RemoteFile.prototype.toString = function () { | ||
return "[" + (this._isDirectory ? "Directory " : "File ") + this._path + "]"; | ||
return "[" + (this._isDirectory ? "Directory " : "RemoteFile ") + this._path + "]"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can a remote URI be described as a directory ? Looks weird to me.
This PR adds File protocol adoption model to support various file protocols which mainly differs in read and write mechanism. With this PR we are adding a reference implementation for this protocol adapter model by enabling
HTTP|HTTPS
protocols. We are introducing a new protocol adapter registration hook inFileSystem
module asfunction registerProtocolAdapter(protocol, adapter)
whereprotocol
can be any valid file protocol string likefor which we want to add support.
adapter
is the the actual protocol adapter object which should exposecanRead(filePath)
function handle andfileImpl
which is theFile
interface implementation prototype handle, this will be be used byFileSystem
to instantiate this new type ofFile
if the protocol adapter is selected based on thefilePath
.This change list also adds a plugin in
QuickOpen
, enabling user to input remote file path (HTTP | HTTPS
) in QuickOpen file search dialog to open a file.TODO