Skip to content

Commit

Permalink
(GH-824)(GH-812) Improve code folding speed (#825)
Browse files Browse the repository at this point in the history
* (maint) Add tests for code folding for PowerShell classes

Previously there were no tests for PowerShell classes.  This commit adds a simple
test for this scenario to ensure future changes do not break folding.

* (GH-824) Refactor the FoldingReference arrays and lists into it's own class

Previously the folding provider created many intermediate arrays and lists and
required post-processing.  This commit changes the behaviour to use an
accumlator patter with an extended Dictionary class.  This new class adds a
`SafeAdd` method to add FoldingRanges, which then has the logic to determine if
the range should indeed be added, for example, passing nulls or pre-existing
larger ranges.

By passing around this list using ByReference we can avoid creating many objects
which are just then thrown away.

This commit also moves the ShowLastLine code from the FoldingProvider into the
Language Server.  This reduces the number of array enumerations to one.

* (GH-824) Refactor Token Folding Operations

Previously each token type detection was separated into discrete blocks
to make reading the code easier. However this meant there were many
enumerations of the Tokens array as well as passing around intermediate
arrays/lists.  This commit takes the individual methods and overlaps them
to reduce the number of enumerations and regular expression matching to
a minimum.

Note that there are considerable code comments here due to the code now
being more complex on initial review.

* (GH-812) Update folder for DSC style scripts

Previously the code folding was not tested against DSC configuration scripts.
This commit adds tests for a sample DSC script to ensure the folding occurs at the correct
places

* (GH-824) More strict block comment folding detection

Previously the folder would search for the region markers without case sensitivity.
This commit modifies the regular expressions to be more strict on the what
is a region marker and adds a negative test to ensure that regions that are not
cased correctly are not folded .
  • Loading branch information
glennsarti authored and TylerLeonhardt committed Jan 15, 2019
1 parent e9445d9 commit 81b3cee
Show file tree
Hide file tree
Showing 4 changed files with 287 additions and 243 deletions.
15 changes: 8 additions & 7 deletions src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ private async Task HandleGetCommandRequestAsync(
{
PSCommand psCommand = new PSCommand();
if (!string.IsNullOrEmpty(param))
{
{
psCommand.AddCommand("Microsoft.PowerShell.Core\\Get-Command").AddArgument(param);
}
else
Expand Down Expand Up @@ -1267,7 +1267,7 @@ protected async Task HandleCodeActionRequest(
}
}

// Add "show documentation" commands last so they appear at the bottom of the client UI.
// Add "show documentation" commands last so they appear at the bottom of the client UI.
// These commands do not require code fixes. Sometimes we get a batch of diagnostics
// to create commands for. No need to create multiple show doc commands for the same rule.
var ruleNamesProcessed = new HashSet<string>();
Expand Down Expand Up @@ -1390,14 +1390,15 @@ private FoldingRange[] Fold(string documentUri)
if (!editorSession.Workspace.TryGetFile(documentUri, out scriptFile)) { return null; }

var result = new List<FoldingRange>();
FoldingReference[] foldableRegions =
TokenOperations.FoldableRegions(scriptFile.ScriptTokens, this.currentSettings.CodeFolding.ShowLastLine);

foreach (FoldingReference fold in foldableRegions)
// If we're showing the last line, decrement the Endline of all regions by one.
int endLineOffset = this.currentSettings.CodeFolding.ShowLastLine ? -1 : 0;

foreach (FoldingReference fold in TokenOperations.FoldableReferences(scriptFile.ScriptTokens).References)
{
result.Add(new FoldingRange {
EndCharacter = fold.EndCharacter,
EndLine = fold.EndLine,
EndLine = fold.EndLine + endLineOffset,
Kind = fold.Kind,
StartCharacter = fold.StartCharacter,
StartLine = fold.StartLine
Expand Down Expand Up @@ -1744,7 +1745,7 @@ await eventSender(
});
}

// Generate a unique id that is used as a key to look up the associated code action (code fix) when
// Generate a unique id that is used as a key to look up the associated code action (code fix) when
// we receive and process the textDocument/codeAction message.
private static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic)
{
Expand Down
49 changes: 49 additions & 0 deletions src/PowerShellEditorServices/Language/FoldingReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//

using System;
using System.Collections.Generic;

namespace Microsoft.PowerShell.EditorServices
{
Expand Down Expand Up @@ -60,4 +61,52 @@ public int CompareTo(FoldingReference that) {
return string.Compare(this.Kind, that.Kind);
}
}

/// <summary>
/// A class that holds a list of FoldingReferences and ensures that when adding a reference that the
/// folding rules are obeyed, e.g. Only one fold per start line
/// </summary>
public class FoldingReferenceList
{
private readonly Dictionary<int, FoldingReference> references = new Dictionary<int, FoldingReference>();

/// <summary>
/// Return all references in the list
/// </summary>
public IEnumerable<FoldingReference> References
{
get
{
return references.Values;
}
}

/// <summary>
/// Adds a FoldingReference to the list and enforces ordering rules e.g. Only one fold per start line
/// </summary>
public void SafeAdd(FoldingReference item)
{
if (item == null) { return; }

// Only add the item if it hasn't been seen before or it's the largest range
if (references.TryGetValue(item.StartLine, out FoldingReference currentItem))
{
if (currentItem.CompareTo(item) == 1) { references[item.StartLine] = item; }
}
else
{
references[item.StartLine] = item;
}
}

/// <summary>
/// Helper method to easily convert the Dictionary Values into an array
/// </summary>
public FoldingReference[] ToArray()
{
var result = new FoldingReference[references.Count];
references.Values.CopyTo(result, 0);
return result;
}
}
}
Loading

0 comments on commit 81b3cee

Please sign in to comment.