Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 518735 - [lsp] textDocument/didSave notifications are being sent …
…early

The textDocument/didSave notification should be sent after the
document has actually been saved to disk. It should also include the
text content of the saved document if the server indicates this
requirement in its initial ServerCapabilities response.

Signed-off-by: Remy Suen <remy.suen@gmail.com>
  • Loading branch information
rcjsuen committed Jun 23, 2017
1 parent 68c9917 commit 9963215
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 8 deletions.
5 changes: 0 additions & 5 deletions bundles/org.eclipse.orion.client.ui/web/orion/editorView.js
Expand Up @@ -607,11 +607,6 @@ define([
if(textView && textView.onSaving) {
textView.onSaving({type:evnt.type});
}
// get the lsp service matching the current content type
var lspLanguageServer = languageServerRegistry.getServerByContentType(inputManager.getContentType());
if (lspLanguageServer) {
lspLanguageServer.didSave(evnt.inputManager.getFileMetadata().Location);
}
});

this.blamer = new mBlamer.Blamer(serviceRegistry, inputManager, editor);
Expand Down
10 changes: 9 additions & 1 deletion bundles/org.eclipse.orion.client.ui/web/orion/inputManager.js
Expand Up @@ -20,8 +20,9 @@ define([
'orion/objects',
'orion/PageUtil',
'orion/editor/textModelFactory',
'orion/lsp/languageServerRegistry',
'orion/metrics'
], function(messages, mNavigatorRenderer, i18nUtil, Deferred, EventTarget, objects, PageUtil, mTextModelFactory, mMetrics) {
], function(messages, mNavigatorRenderer, i18nUtil, Deferred, EventTarget, objects, PageUtil, mTextModelFactory, mLanguageServerRegistry, mMetrics) {

function Idle(options){
this._document = options.document || document;
Expand Down Expand Up @@ -146,6 +147,7 @@ define([
this.generalPreferences = options.generalPreferences || {};
this.isEditorTabsEnabled = options.isEditorTabsEnabled || false;
this._input = this._title = "";
this.languageServerRegistry = new mLanguageServerRegistry.LanguageServerRegistry(this.serviceRegistry, options.problemsServiceID || "orion.core.marker"); //$NON-NLS-0$
if (this.fileClient) {
this.fileClient.addEventListener("Changed", function(evt) { //$NON-NLS-0$
if (this._fileMetadata && this._fileMetadata._saving) {
Expand Down Expand Up @@ -442,6 +444,12 @@ define([
if (that.postSave) {
that.postSave(closing);
}
// get the lsp service matching the current content type
var lspLanguageServer = that.languageServerRegistry.getServerByContentType(that.getContentType());
if (lspLanguageServer) {
var text = lspLanguageServer.includeTextOnSave() ? that.getEditor().getText() : undefined;
lspLanguageServer.didSave(that.getFileMetadata().Location, text);
}
return done(result);
}
function errorHandler(error) {
Expand Down
6 changes: 4 additions & 2 deletions bundles/org.eclipse.orion.client.ui/web/orion/lsp/ipc.js
Expand Up @@ -360,12 +360,14 @@ define([], function() {
* @description The document save notification is sent from the client to the server when the document was saved in the client.
* @function
* @param {String} uri The URI of the file
* @param {String} [text] the text content of the saved document
*/
IPC.prototype.didSave = function didSave(uri) {
IPC.prototype.didSave = function didSave(uri, text) {
return this.sendMessage(0, this.MESSAGE_TYPES.didSave, {
textDocument: {
uri: uri,
}
},
text: text
});
};

Expand Down
Expand Up @@ -128,6 +128,21 @@ define([
return this.ipc.TEXT_DOCUMENT_SYNC_KIND.None;
},

/**
* Returns whether the content of the saved document should be included in a
* 'textDocument/didSave' notification to the server.
*
* @return {boolean} true if the document's text content should be included
* with the 'textDocument/didSave' notification to the
* server, false otherwise
*/
includeTextOnSave: function() {
if (this.capabilities && this.capabilities.textDocumentSync && this.capabilities.textDocumentSync.save) {
return this.capabilities.textDocumentSync.save.includeText === true;
}
return false;
},

isDefinitionEnabled: function() {
return this.capabilities && this.capabilities.definitionProvider;
},
Expand Down

0 comments on commit 9963215

Please sign in to comment.