Skip to content

add Save to FileContext API #590

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 3, 2018
Merged

add Save to FileContext API #590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/PowerShellEditorServices.Protocol/LanguageServer/EditorCommands.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,13 @@ public static readonly
RequestType<string, EditorCommandResponse, object, object>.Create("editor/closeFile");
}

public class SaveFileRequest
{
public static readonly
RequestType<string, EditorCommandResponse, object, object> Type =
RequestType<string, EditorCommandResponse, object, object>.Create("editor/saveFile");
}

public class ShowInformationMessageRequest
{
public static readonly
Expand Down
9 changes: 9 additions & 0 deletions src/PowerShellEditorServices.Protocol/Server/LanguageServerEditorOperations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,15 @@ public Task CloseFile(string filePath)
true);
}

public Task SaveFile(string filePath)
{
return
this.messageSender.SendRequest(
SaveFileRequest.Type,
filePath,
true);
}

public string GetWorkspacePath()
{
return this.editorSession.Workspace.WorkspacePath;
Expand Down
12 changes: 12 additions & 0 deletions src/PowerShellEditorServices/Extensions/FileContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,18 @@ public void InsertText(string textToInsert, BufferRange insertRange)
}

#endregion

#region File Manipulation

/// <summary>
/// Saves this file.
/// </summary>
public void Save()
{
this.editorOperations.SaveFile(this.scriptFile.FilePath);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you pass ClientFilePath here instead of FilePath you should be able to avoid resolving the path when it gets to VSCode. The InsertText method is a good example for both sides.

Copy link
Member Author

@TylerLeonhardt TylerLeonhardt Dec 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SeeminglyScience + @daviwil: So I tried passing in ClientFilePath. It prefaced it with file://.

This is not ideal because for openFile and closeFile here:
https://github.com/PowerShell/vscode-powershell/blob/master/src/features/ExtensionCommands.ts#L362-L409

We have VSCode already resolving the paths.

So I think we should either continue resolving the paths via VSCode, or modify openFile and closeFile to expect a ClientFilePath as well. That way, we are consistent.

Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tylerl0706 Ah I'd keep it as is then. In general I think it's better to have PSES do the work, but it's probably not worth changing the others.

}

#endregion
}
}

7 changes: 7 additions & 0 deletions src/PowerShellEditorServices/Extensions/IEditorOperations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ public interface IEditorOperations
/// <returns>A Task that can be tracked for completion.</returns>
Task CloseFile(string filePath);

/// <summary>
/// Causes a file to be saved in the editor.
/// </summary>
/// <param name="filePath">The path of the file to be saved.</param>
/// <returns>A Task that can be tracked for completion.</returns>
Task SaveFile(string filePath);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would adding to this interface be a breaking change? I know we have at least two new implementations of PSES since this interface was last changed. Should new features be added as additional interfaces, something like IEditorSaveOperations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.. but I think if we were going to make a new interface, I'd want it to be IEditorFileOperations and it would contain NewFile, OpenFile, CloseFile, SaveFile`, ya know? And that would surely warrant a breaking change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, nobody is using these interfaces outside of PSES itself. Adding a method to an interface wouldn't be a breaking change so I think a new interface shouldn't be necessary for this.

/// <summary>
/// Inserts text into the specified range for the file at the specified path.
/// </summary>
Expand Down
5 changes: 5 additions & 0 deletions test/PowerShellEditorServices.Test/Extensions/ExtensionServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@ public Task CloseFile(string filePath)
throw new NotImplementedException();
}

public Task SaveFile(string filePath)
{
throw new NotImplementedException();
}

public Task InsertText(string filePath, string text, BufferRange insertRange)
{
throw new NotImplementedException();
Expand Down