Skip to content

(GH-793) Add syntax folding #777

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

Merged
merged 4 commits into from
Nov 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
69 changes: 69 additions & 0 deletions src/PowerShellEditorServices.Protocol/LanguageServer/Folding.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

using Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol;

namespace Microsoft.PowerShell.EditorServices.Protocol.LanguageServer
{
public class FoldingRangeRequest
{
/// <summary>
/// A request to provide folding ranges in a document. The request's
/// parameter is of type [FoldingRangeParams](#FoldingRangeParams), the
/// response is of type [FoldingRangeList](#FoldingRangeList) or a Thenable
/// that resolves to such.
/// Ref: https://github.com/Microsoft/vscode-languageserver-node/blob/5350bc2ffe8afb17357c1a66fbdd3845fa05adfd/protocol/src/protocol.foldingRange.ts#L112-L120
/// </summary>
public static readonly
RequestType<FoldingRangeParams, FoldingRange[], object, object> Type =
RequestType<FoldingRangeParams, FoldingRange[], object, object>.Create("textDocument/foldingRange");
}

/// <summary>
/// Parameters for a [FoldingRangeRequest](#FoldingRangeRequest).
/// Ref: https://github.com/Microsoft/vscode-languageserver-node/blob/5350bc2ffe8afb17357c1a66fbdd3845fa05adfd/protocol/src/protocol.foldingRange.ts#L102-L110
/// </summary>
public class FoldingRangeParams
{
/// <summary>
/// The text document
/// </summary>
public TextDocumentIdentifier TextDocument { get; set; }
}

/// <summary>
/// Represents a folding range.
/// Ref: https://github.com/Microsoft/vscode-languageserver-node/blob/5350bc2ffe8afb17357c1a66fbdd3845fa05adfd/protocol/src/protocol.foldingRange.ts#L69-L100
/// </summary>
public class FoldingRange
{
/// <summary>
/// The zero-based line number from where the folded range starts.
/// </summary>
public int StartLine { get; set; }

/// <summary>
/// The zero-based character offset from where the folded range starts. If not defined, defaults to the length of the start line.
/// </summary>
public int StartCharacter { get; set; }

/// <summary>
/// The zero-based line number where the folded range ends.
/// </summary>
public int EndLine { get; set; }

/// <summary>
/// The zero-based character offset before the folded range ends. If not defined, defaults to the length of the end line.
/// </summary>
public int EndCharacter { get; set; }

/// <summary>
/// Describes the kind of the folding range such as `comment' or 'region'. The kind
/// is used to categorize folding ranges and used by commands like 'Fold all comments'. See
/// [FoldingRangeKind](#FoldingRangeKind) for an enumeration of standardized kinds.
/// </summary>
public string Kind { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public class ServerCapabilities
public ExecuteCommandOptions ExecuteCommandProvider { get; set; }

public object Experimental { get; set; }

public bool FoldingRangeProvider { get; set; } = false;
Copy link
Member

Choose a reason for hiding this comment

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

bool defaults to false so the = false shouldn't be needed I think. Since bool is a non-nullable type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO, in this case, being explicit makes it more obvious what's going on. I don't feel there's any maintenance cost with leaving this in.

Copy link
Member

Choose a reason for hiding this comment

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

Works for me 👍

}

/// <summary>
Expand Down
32 changes: 31 additions & 1 deletion src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ public void Start()
this.messageHandlers.SetRequestHandler(
DocumentRangeFormattingRequest.Type,
this.HandleDocumentRangeFormattingRequest);
this.messageHandlers.SetRequestHandler(FoldingRangeRequest.Type, this.HandleFoldingRangeRequestAsync);

this.messageHandlers.SetRequestHandler(ShowOnlineHelpRequest.Type, this.HandleShowOnlineHelpRequest);
this.messageHandlers.SetRequestHandler(ShowHelpRequest.Type, this.HandleShowHelpRequest);
Expand Down Expand Up @@ -239,7 +240,8 @@ await requestContext.SendResult(
},
DocumentFormattingProvider = false,
DocumentRangeFormattingProvider = false,
RenameProvider = false
RenameProvider = false,
FoldingRangeProvider = true
}
});
}
Expand Down Expand Up @@ -1250,6 +1252,13 @@ await requestContext.SendResult(new TextEdit[1]
});
}

protected async Task HandleFoldingRangeRequestAsync(
FoldingRangeParams foldingParams,
RequestContext<FoldingRange[]> requestContext)
{
await requestContext.SendResult(Fold(foldingParams.TextDocument.Uri));
}

protected Task HandleEvaluateRequest(
DebugAdapterMessages.EvaluateRequestArguments evaluateParams,
RequestContext<DebugAdapterMessages.EvaluateResponseBody> requestContext)
Expand Down Expand Up @@ -1288,6 +1297,27 @@ protected Task HandleEvaluateRequest(

#region Event Handlers

private FoldingRange[] Fold(
string documentUri)
{
// TODO Should be using dynamic registrations
if (!this.currentSettings.CodeFolding.Enable) { return null; }
var result = new List<FoldingRange>();
foreach (FoldingReference fold in TokenOperations.FoldableRegions(
editorSession.Workspace.GetFile(documentUri).ScriptTokens,
this.currentSettings.CodeFolding.ShowLastLine))
{
result.Add(new FoldingRange {
EndCharacter = fold.EndCharacter,
EndLine = fold.EndLine,
Kind = fold.Kind,
StartCharacter = fold.StartCharacter,
StartLine = fold.StartLine
});
}
return result.ToArray();
}

private async Task<Tuple<string, Range>> Format(
string documentUri,
FormattingOptions options,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ public class LanguageServerSettings

public CodeFormattingSettings CodeFormatting { get; set; }

public CodeFoldingSettings CodeFolding { get; set; }

public LanguageServerSettings()
{
this.ScriptAnalysis = new ScriptAnalysisSettings();
this.CodeFormatting = new CodeFormattingSettings();
this.CodeFolding = new CodeFoldingSettings();
}

public void Update(
Expand All @@ -39,6 +42,7 @@ public void Update(
workspaceRootPath,
logger);
this.CodeFormatting = new CodeFormattingSettings(settings.CodeFormatting);
this.CodeFolding.Update(settings.CodeFolding, logger);
}
}
}
Expand Down Expand Up @@ -261,6 +265,41 @@ private Hashtable GetCustomPSSASettingsHashtable(int tabSize, bool insertSpaces)
}
}

/// <summary>
/// Code folding settings
/// </summary>
public class CodeFoldingSettings
{
/// <summary>
/// Whether the folding is enabled. Default is true as per VSCode
/// </summary>
public bool Enable { get; set; } = true;

/// <summary>
/// Whether to show or hide the last line of a folding region. Default is true as per VSCode
/// </summary>
public bool ShowLastLine { get; set; } = true;

/// <summary>
/// Update these settings from another settings object
/// </summary>
public void Update(
CodeFoldingSettings settings,
ILogger logger)
{
if (settings != null) {
if (this.Enable != settings.Enable) {
this.Enable = settings.Enable;
logger.Write(LogLevel.Verbose, string.Format("Using Code Folding Enabled - {0}", this.Enable));
}
if (this.ShowLastLine != settings.ShowLastLine) {
this.ShowLastLine = settings.ShowLastLine;
logger.Write(LogLevel.Verbose, string.Format("Using Code Folding ShowLastLine - {0}", this.ShowLastLine));
}
}
}
}

public class LanguageServerSettingsWrapper
{
// NOTE: This property is capitalized as 'Powershell' because the
Expand Down
63 changes: 63 additions & 0 deletions src/PowerShellEditorServices/Language/FoldingReference.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

using System;

namespace Microsoft.PowerShell.EditorServices
{
/// <summary>
/// A class that holds the information for a foldable region of text in a document
/// </summary>
public class FoldingReference: IComparable<FoldingReference>
{
/// <summary>
/// The zero-based line number from where the folded range starts.
/// </summary>
public int StartLine { get; set; }

/// <summary>
/// The zero-based character offset from where the folded range starts. If not defined, defaults to the length of the start line.
/// </summary>
public int StartCharacter { get; set; } = 0;

/// <summary>
/// The zero-based line number where the folded range ends.
/// </summary>
public int EndLine { get; set; }

/// <summary>
/// The zero-based character offset before the folded range ends. If not defined, defaults to the length of the end line.
/// </summary>
public int EndCharacter { get; set; } = 0;

/// <summary>
/// Describes the kind of the folding range such as `comment' or 'region'.
/// </summary>
public string Kind { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this based on the VSCode type? Would it be better as an enum? We could extend that as easily as a string, but it's faster to check, less error prone and can't be null.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that the VSCode API can have this as null, a common convention in C# is to have a None value in the enum:

enum MatchKind
{
    None = 0,
    Comment,
    Region
}

(Or similar)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See discussion above about strings and enums. This type is String (https://github.com/Microsoft/vscode-languageserver-node/blob/master/protocol/src/protocol.foldingRange.ts#L94-L99)

With 3 (at this time) well known strings (https://github.com/Microsoft/vscode-languageserver-node/blob/master/protocol/src/protocol.foldingRange.ts#L51-L67)

 * Enum of known range kinds
 */
export enum FoldingRangeKind {
	/**
	 * Folding range for a comment
	 */
	Comment = 'comment',
	/**
	 * Folding range for a imports or includes
	 */
	Imports = 'imports',
	/**
	 * Folding range for a region (e.g. `#region`)
	 */
	Region = 'region'
}

I couldn't use an enum as C# enums are ints (well, ordinal) not strings. I later just used string constants as I only needed it in one place

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it because it's a JSON deserialisation thing? If so we should take a quick look at how we deserialise messages and see if we can use [JsonConverter(typeof(StringEnumConverter))] or something like that on the type. Being able to use enums would just be really nice here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case it's serialisation only. We don't receive this from a client.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I'm good with how we do this. I looked into restructuring it to allow enums, but given that it took several changes in various places, I might save it to go with some other work I've been wanting to do.


/// <summary>
/// A custom comparable method which can properly sort FoldingReference objects
/// </summary>
public int CompareTo(FoldingReference that) {
// Initially look at the start line
if (this.StartLine < that.StartLine) { return -1; }
if (this.StartLine > that.StartLine) { return 1; }

// They have the same start line so now consider the end line.
// The biggest line range is sorted first
if (this.EndLine > that.EndLine) { return -1; }
if (this.EndLine < that.EndLine) { return 1; }

// They have the same lines, but what about character offsets
if (this.StartCharacter < that.StartCharacter) { return -1; }
if (this.StartCharacter > that.StartCharacter) { return 1; }
if (this.EndCharacter < that.EndCharacter) { return -1; }
if (this.EndCharacter > that.EndCharacter) { return 1; }

// They're the same range, but what about kind
return string.Compare(this.Kind, that.Kind);
}
}
}
Loading