Skip to content

Commit fc65ffe

Browse files
committed
(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.
1 parent bfe7e0a commit fc65ffe

File tree

4 files changed

+108
-125
lines changed

4 files changed

+108
-125
lines changed

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+11-6
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ private async Task HandleGetCommandRequestAsync(
535535
{
536536
PSCommand psCommand = new PSCommand();
537537
if (!string.IsNullOrEmpty(param))
538-
{
538+
{
539539
psCommand.AddCommand("Microsoft.PowerShell.Core\\Get-Command").AddArgument(param);
540540
}
541541
else
@@ -1267,7 +1267,7 @@ protected async Task HandleCodeActionRequest(
12671267
}
12681268
}
12691269

1270-
// Add "show documentation" commands last so they appear at the bottom of the client UI.
1270+
// Add "show documentation" commands last so they appear at the bottom of the client UI.
12711271
// These commands do not require code fixes. Sometimes we get a batch of diagnostics
12721272
// to create commands for. No need to create multiple show doc commands for the same rule.
12731273
var ruleNamesProcessed = new HashSet<string>();
@@ -1382,13 +1382,18 @@ private FoldingRange[] Fold(
13821382
// TODO Should be using dynamic registrations
13831383
if (!this.currentSettings.CodeFolding.Enable) { return null; }
13841384
var result = new List<FoldingRange>();
1385+
1386+
ScriptFile script = editorSession.Workspace.GetFile(documentUri);
1387+
1388+
// If we're showing the last line, decrement the Endline of all regions by one.
1389+
int endLineOffset = this.currentSettings.CodeFolding.ShowLastLine ? -1 : 0;
1390+
13851391
foreach (FoldingReference fold in TokenOperations.FoldableRegions(
1386-
editorSession.Workspace.GetFile(documentUri).ScriptTokens,
1387-
this.currentSettings.CodeFolding.ShowLastLine))
1392+
script.ScriptTokens).ToArray())
13881393
{
13891394
result.Add(new FoldingRange {
13901395
EndCharacter = fold.EndCharacter,
1391-
EndLine = fold.EndLine,
1396+
EndLine = fold.EndLine + endLineOffset,
13921397
Kind = fold.Kind,
13931398
StartCharacter = fold.StartCharacter,
13941399
StartLine = fold.StartLine
@@ -1734,7 +1739,7 @@ await eventSender(
17341739
});
17351740
}
17361741

1737-
// Generate a unique id that is used as a key to look up the associated code action (code fix) when
1742+
// Generate a unique id that is used as a key to look up the associated code action (code fix) when
17381743
// we receive and process the textDocument/codeAction message.
17391744
private static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic)
17401745
{

src/PowerShellEditorServices/Language/FoldingReference.cs

+36
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//
55

66
using System;
7+
using System.Collections.Generic;
78

89
namespace Microsoft.PowerShell.EditorServices
910
{
@@ -60,4 +61,39 @@ public int CompareTo(FoldingReference that) {
6061
return string.Compare(this.Kind, that.Kind);
6162
}
6263
}
64+
65+
/// <summary>
66+
/// A class that holds a list of FoldingReferences and ensures that when adding a reference that the
67+
/// folding rules are obeyed, e.g. Only one fold per start line
68+
/// </summary>
69+
public class FoldingReferenceList : Dictionary<int, FoldingReference>
70+
{
71+
/// <summary>
72+
/// Adds a FoldingReference to the list and enforces ordering rules e.g. Only one fold per start line
73+
/// </summary>
74+
public void SafeAdd(FoldingReference item)
75+
{
76+
if (item == null) { return; }
77+
78+
// Only add the item if it hasn't been seen before or it's the largest range
79+
if (TryGetValue(item.StartLine, out FoldingReference currentItem))
80+
{
81+
if (currentItem.CompareTo(item) == 1) { this[item.StartLine] = item; }
82+
}
83+
else
84+
{
85+
this[item.StartLine] = item;
86+
}
87+
}
88+
89+
/// <summary>
90+
/// Helper method to easily convert the Dictionary Values into an array
91+
/// </summary>
92+
public FoldingReference[] ToArray()
93+
{
94+
var result = new FoldingReference[Count];
95+
Values.CopyTo(result, 0);
96+
return result;
97+
}
98+
}
6399
}

src/PowerShellEditorServices/Language/TokenOperations.cs

+29-79
Original file line numberDiff line numberDiff line change
@@ -39,85 +39,39 @@ internal static class TokenOperations
3939
/// <summary>
4040
/// Extracts all of the unique foldable regions in a script given the list tokens
4141
/// </summary>
42-
internal static FoldingReference[] FoldableRegions(
43-
Token[] tokens,
44-
bool ShowLastLine)
42+
internal static FoldingReferenceList FoldableRegions(
43+
Token[] tokens)
4544
{
46-
List<FoldingReference> foldableRegions = new List<FoldingReference>();
45+
var refList = new FoldingReferenceList();
4746

4847
// Find matching braces { -> }
4948
// Find matching hashes @{ -> }
50-
foldableRegions.AddRange(
51-
MatchTokenElements(tokens, s_openingBraces, TokenKind.RCurly, RegionKindNone)
52-
);
49+
MatchTokenElements(tokens, s_openingBraces, TokenKind.RCurly, RegionKindNone, ref refList);
5350

5451
// Find matching parentheses ( -> )
5552
// Find matching array literals @( -> )
5653
// Find matching subexpressions $( -> )
57-
foldableRegions.AddRange(
58-
MatchTokenElements(tokens, s_openingParens, TokenKind.RParen, RegionKindNone)
59-
);
54+
MatchTokenElements(tokens, s_openingParens, TokenKind.RParen, RegionKindNone, ref refList);
6055

6156
// Find contiguous here strings @' -> '@
62-
foldableRegions.AddRange(
63-
MatchTokenElement(tokens, TokenKind.HereStringLiteral, RegionKindNone)
64-
);
57+
MatchTokenElement(tokens, TokenKind.HereStringLiteral, RegionKindNone, ref refList);
6558

6659
// Find unopinionated variable names ${ \n \n }
67-
foldableRegions.AddRange(
68-
MatchTokenElement(tokens, TokenKind.Variable, RegionKindNone)
69-
);
60+
MatchTokenElement(tokens, TokenKind.Variable, RegionKindNone, ref refList);
7061

7162
// Find contiguous here strings @" -> "@
72-
foldableRegions.AddRange(
73-
MatchTokenElement(tokens, TokenKind.HereStringExpandable, RegionKindNone)
74-
);
63+
MatchTokenElement(tokens, TokenKind.HereStringExpandable, RegionKindNone, ref refList);
7564

7665
// Find matching comment regions #region -> #endregion
77-
foldableRegions.AddRange(
78-
MatchCustomCommentRegionTokenElements(tokens, RegionKindRegion)
79-
);
66+
MatchCustomCommentRegionTokenElements(tokens, RegionKindRegion, ref refList);
8067

8168
// Find blocks of line comments # comment1\n# comment2\n...
82-
foldableRegions.AddRange(
83-
MatchBlockCommentTokenElement(tokens, RegionKindComment)
84-
);
69+
MatchBlockCommentTokenElement(tokens, RegionKindComment, ref refList);
8570

8671
// Find comments regions <# -> #>
87-
foldableRegions.AddRange(
88-
MatchTokenElement(tokens, TokenKind.Comment, RegionKindComment)
89-
);
90-
91-
// Remove any null entries. Nulls appear if the folding reference is invalid
92-
// or missing
93-
foldableRegions.RemoveAll(item => item == null);
94-
95-
// Sort the FoldingReferences, starting at the top of the document,
96-
// and ensure that, in the case of multiple ranges starting the same line,
97-
// that the largest range (i.e. most number of lines spanned) is sorted
98-
// first. This is needed to detect duplicate regions. The first in the list
99-
// will be used and subsequent duplicates ignored.
100-
foldableRegions.Sort();
101-
102-
// It's possible to have duplicate or overlapping ranges, that is, regions which have the same starting
103-
// line number as the previous region. Therefore only emit ranges which have a different starting line
104-
// than the previous range.
105-
foldableRegions.RemoveAll( (FoldingReference item) => {
106-
// Note - I'm not happy with searching here, but as the RemoveAll
107-
// doesn't expose the index in the List, we need to calculate it. Fortunately the
108-
// list is sorted at this point, so we can use BinarySearch.
109-
int index = foldableRegions.BinarySearch(item);
110-
if (index == 0) { return false; }
111-
return (item.StartLine == foldableRegions[index - 1].StartLine);
112-
});
113-
114-
// Some editors have different folding UI, sometimes the lastline should be displayed
115-
// If we do want to show the last line, just change the region to be one line less
116-
if (ShowLastLine) {
117-
foldableRegions.ForEach( item => { item.EndLine--; });
118-
}
72+
MatchTokenElement(tokens, TokenKind.Comment, RegionKindComment, ref refList);
11973

120-
return foldableRegions.ToArray();
74+
return refList;
12175
}
12276

12377
/// <summary>
@@ -163,42 +117,40 @@ static private FoldingReference CreateFoldingReference(
163117
/// <summary>
164118
/// Given an array of tokens, find matching regions which start (array of tokens) and end with a different TokenKind
165119
/// </summary>
166-
static private List<FoldingReference> MatchTokenElements(
120+
static private void MatchTokenElements(
167121
Token[] tokens,
168122
TokenKind[] startTokenKind,
169123
TokenKind endTokenKind,
170-
string matchKind)
124+
string matchKind,
125+
ref FoldingReferenceList refList)
171126
{
172-
List<FoldingReference> result = new List<FoldingReference>();
173127
Stack<Token> tokenStack = new Stack<Token>();
174128
foreach (Token token in tokens)
175129
{
176130
if (Array.IndexOf(startTokenKind, token.Kind) != -1) {
177131
tokenStack.Push(token);
178132
}
179133
if ((tokenStack.Count > 0) && (token.Kind == endTokenKind)) {
180-
result.Add(CreateFoldingReference(tokenStack.Pop(), token, matchKind));
134+
refList.SafeAdd(CreateFoldingReference(tokenStack.Pop(), token, matchKind));
181135
}
182136
}
183-
return result;
184137
}
185138

186139
/// <summary>
187140
/// Given an array of token, finds a specific token
188141
/// </summary>
189-
static private List<FoldingReference> MatchTokenElement(
142+
static private void MatchTokenElement(
190143
Token[] tokens,
191144
TokenKind tokenKind,
192-
string matchKind)
145+
string matchKind,
146+
ref FoldingReferenceList refList)
193147
{
194-
List<FoldingReference> result = new List<FoldingReference>();
195148
foreach (Token token in tokens)
196149
{
197150
if ((token.Kind == tokenKind) && (token.Extent.StartLineNumber != token.Extent.EndLineNumber)) {
198-
result.Add(CreateFoldingReference(token, token, matchKind));
151+
refList.SafeAdd(CreateFoldingReference(token, token, matchKind));
199152
}
200153
}
201-
return result;
202154
}
203155

204156
/// <summary>
@@ -230,11 +182,11 @@ static private bool IsBlockComment(int index, Token[] tokens) {
230182
/// classed as comments. To workaround this we search for valid block comments (See IsBlockCmment)
231183
/// and then determine contiguous line numbers from there
232184
/// </summary>
233-
static private List<FoldingReference> MatchBlockCommentTokenElement(
185+
static private void MatchBlockCommentTokenElement(
234186
Token[] tokens,
235-
string matchKind)
187+
string matchKind,
188+
ref FoldingReferenceList refList)
236189
{
237-
List<FoldingReference> result = new List<FoldingReference>();
238190
Token startToken = null;
239191
int nextLine = -1;
240192
for (int index = 0; index < tokens.Length; index++)
@@ -243,7 +195,7 @@ static private List<FoldingReference> MatchBlockCommentTokenElement(
243195
if (IsBlockComment(index, tokens) && s_nonRegionLineCommentRegex.IsMatch(thisToken.Text)) {
244196
int thisLine = thisToken.Extent.StartLineNumber - 1;
245197
if ((startToken != null) && (thisLine != nextLine)) {
246-
result.Add(CreateFoldingReference(startToken, nextLine - 1, matchKind));
198+
refList.SafeAdd(CreateFoldingReference(startToken, nextLine - 1, matchKind));
247199
startToken = thisToken;
248200
}
249201
if (startToken == null) { startToken = thisToken; }
@@ -253,27 +205,26 @@ static private List<FoldingReference> MatchBlockCommentTokenElement(
253205
// If we exit the token array and we're still processing comment lines, then the
254206
// comment block simply ends at the end of document
255207
if (startToken != null) {
256-
result.Add(CreateFoldingReference(startToken, nextLine - 1, matchKind));
208+
refList.SafeAdd(CreateFoldingReference(startToken, nextLine - 1, matchKind));
257209
}
258-
return result;
259210
}
260211

261212
/// <summary>
262213
/// Given a list of tokens, find the tokens that are comments and
263214
/// the comment text is either `# region` or `# endregion`, and then use a stack to determine
264215
/// the ranges they span
265216
/// </summary>
266-
static private List<FoldingReference> MatchCustomCommentRegionTokenElements(
217+
static private void MatchCustomCommentRegionTokenElements(
267218
Token[] tokens,
268-
string matchKind)
219+
string matchKind,
220+
ref FoldingReferenceList refList)
269221
{
270222
// These regular expressions are used to match lines which mark the start and end of region comment in a PowerShell
271223
// script. They are based on the defaults in the VS Code Language Configuration at;
272224
// https://github.com/Microsoft/vscode/blob/64186b0a26/extensions/powershell/language-configuration.json#L26-L31
273225
string startRegionText = @"^\s*#region\b";
274226
string endRegionText = @"^\s*#endregion\b";
275227

276-
List<FoldingReference> result = new List<FoldingReference>();
277228
Stack<Token> tokenStack = new Stack<Token>();
278229
for (int index = 0; index < tokens.Length; index++)
279230
{
@@ -283,11 +234,10 @@ static private List<FoldingReference> MatchCustomCommentRegionTokenElements(
283234
tokenStack.Push(token);
284235
}
285236
if ((tokenStack.Count > 0) && (Regex.IsMatch(token.Text, endRegionText, RegexOptions.IgnoreCase))) {
286-
result.Add(CreateFoldingReference(tokenStack.Pop(), token, matchKind));
237+
refList.SafeAdd(CreateFoldingReference(tokenStack.Pop(), token, matchKind));
287238
}
288239
}
289240
}
290-
return result;
291241
}
292242
}
293243
}

0 commit comments

Comments
 (0)