-
Notifications
You must be signed in to change notification settings - Fork 418
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
Add a /blockstructure endpoint that uses Roslyn's outlining implemention #1209
Changes from 3 commits
ab2a160
8055a70
163336d
2b675c3
e57dbc4
1b76656
53828f6
27d5eb3
a1c7841
6d94006
be90f02
af6ee65
473500b
d8dbd6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
namespace OmniSharp.Models.V2 | ||
{ | ||
public class CodeFoldingBlock | ||
{ | ||
public CodeFoldingBlock(Range textSpan, string type) | ||
{ | ||
Range = textSpan; | ||
Type = type; | ||
} | ||
|
||
/// <summary> | ||
/// The span of text to collapse. | ||
/// </summary> | ||
public Range Range { get; } | ||
|
||
/// <summary> | ||
/// If the block is one of the types specified in <see cref="CodeFoldingBlockKind"/>, that type. | ||
/// Otherwise, null. | ||
/// </summary> | ||
public string Type { get; } | ||
} | ||
|
||
public class CodeFoldingBlockKind | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably be plural: |
||
{ | ||
public static readonly string Comment = nameof(Comment).ToLowerInvariant(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I chose to include these values because the VS Code folding API has special handling for these types of blocks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! The type should probably be plural -- |
||
public static readonly string Imports = nameof(Imports).ToLowerInvariant(); | ||
public static readonly string Region = nameof(Region).ToLowerInvariant(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,12 +12,12 @@ public class Point : IEquatable<Point> | |
public int Column { get; set; } | ||
|
||
public override bool Equals(object obj) | ||
{ | ||
var point = obj as Point; | ||
return point != null && | ||
Line == point.Line && | ||
Column == point.Column; | ||
} | ||
=> Equals(obj as Point); | ||
|
||
public bool Equals(Point other) | ||
=> other != null | ||
&& Line == other.Line | ||
&& Column == other.Column; | ||
|
||
public override int GetHashCode() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will conflict with my PR. Sorry about that! |
||
{ | ||
|
@@ -26,5 +26,14 @@ public override int GetHashCode() | |
hashCode = hashCode * -1521134295 + Column.GetHashCode(); | ||
return hashCode; | ||
} | ||
|
||
public override string ToString() | ||
=> $"Line = {Line}, Column = {Column}"; | ||
|
||
public static bool operator ==(Point point1, Point point2) | ||
=> EqualityComparer<Point>.Default.Equals(point1, point2); | ||
|
||
public static bool operator !=(Point point1, Point point2) | ||
=> !(point1 == point2); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
|
||
namespace OmniSharp.Models.V2 | ||
|
@@ -7,20 +8,49 @@ public class Range : IEquatable<Range> | |
public Point Start { get; set; } | ||
public Point End { get; set; } | ||
|
||
public override bool Equals(object obj) | ||
public bool Contains(int line, int column) | ||
{ | ||
var range = obj as Range; | ||
return range != null && | ||
EqualityComparer<Point>.Default.Equals(Start, range.Start) && | ||
EqualityComparer<Point>.Default.Equals(End, range.End); | ||
if (Start.Line > line || End.Line < line) | ||
{ | ||
return false; | ||
} | ||
|
||
if (Start.Line == line && Start.Column > column) | ||
{ | ||
return false; | ||
} | ||
|
||
if (End.Line == line && End.Column < column) | ||
{ | ||
return false; | ||
} | ||
|
||
return true; | ||
} | ||
|
||
public override bool Equals(object obj) | ||
=> Equals(obj as Range); | ||
|
||
public bool Equals(Range other) | ||
=> other != null | ||
&& EqualityComparer<Point>.Default.Equals(Start, other.Start) | ||
&& EqualityComparer<Point>.Default.Equals(End, other.End); | ||
|
||
public override int GetHashCode() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
{ | ||
var hashCode = -1676728671; | ||
hashCode = hashCode * -1521134295 + EqualityComparer<Point>.Default.GetHashCode(Start); | ||
hashCode = hashCode * -1521134295 + EqualityComparer<Point>.Default.GetHashCode(End); | ||
return hashCode; | ||
} | ||
|
||
public override string ToString() | ||
=> $"Start = {{{Start}}}, End = {{{End}}}"; | ||
|
||
public static bool operator ==(Range range1, Range range2) | ||
=> EqualityComparer<Range>.Default.Equals(range1, range2); | ||
|
||
public static bool operator !=(Range range1, Range range2) | ||
=> !(range1 == range2); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,7 +47,6 @@ public static LanguageVersion ToLanguageVersion(string propertyValue) | |
case "7": return LanguageVersion.CSharp7; | ||
case "7.1": return LanguageVersion.CSharp7_1; | ||
case "7.2": return LanguageVersion.CSharp7_2; | ||
case "7.3": return LanguageVersion.CSharp7_3; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you meant to break C# 7.3 did you? Mis-merge? |
||
default: return LanguageVersion.Default; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,4 +16,8 @@ | |
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(MicrosoftCodeAnalysisCSharpWorkspacesVersion)" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<Folder Include="Services\Blocks\" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks unnecessary. |
||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,10 +7,10 @@ | |
using System.Threading.Tasks; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort with System namespaces on top. |
||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.Text; | ||
using OmniSharp.Extensions; | ||
using OmniSharp.Mef; | ||
using OmniSharp.Models.v2; | ||
using OmniSharp.Models.V2; | ||
using OmniSharp.Roslyn.Extensions; | ||
using OmniSharp.Services; | ||
using OmniSharp.Utilities; | ||
|
||
|
@@ -28,8 +28,6 @@ public class BlockStructureService : IRequestHandler<BlockStructureRequest, Bloc | |
private readonly MethodInfo _getSpans; | ||
private readonly MethodInfo _getIsCollpasible; | ||
private readonly MethodInfo _getTextSpan; | ||
private readonly MethodInfo _getHintSpan; | ||
private readonly MethodInfo _getBannerText; | ||
private readonly MethodInfo _getType; | ||
private readonly OmniSharpWorkspace _workspace; | ||
|
||
|
@@ -48,45 +46,43 @@ public BlockStructureService(IAssemblyLoader loader, OmniSharpWorkspace workspac | |
_getSpans = _blockStructure.Value.GetProperty("Spans").GetMethod; | ||
_getIsCollpasible = _blockSpan.Value.GetProperty("IsCollapsible").GetMethod; | ||
_getTextSpan = _blockSpan.Value.GetProperty("TextSpan").GetMethod; | ||
_getHintSpan = _blockSpan.Value.GetProperty("HintSpan").GetMethod; | ||
_getBannerText = _blockSpan.Value.GetProperty("BannerText").GetMethod; | ||
_getType = _blockSpan.Value.GetProperty("Type").GetMethod; | ||
} | ||
|
||
public async Task<BlockStructureResponse> Handle(BlockStructureRequest request) | ||
{ | ||
var document = _workspace.GetDocument(request.FileName); | ||
var text = await document.GetTextAsync(); | ||
var lines = text.Lines; | ||
|
||
var service = _blockStructureService.LazyGetMethod("GetService").InvokeStatic(new[] { document }); | ||
|
||
var structure = _getBlockStructure.Invoke<object>(service, new object[] { document, CancellationToken.None }); | ||
var spans = _getSpans.Invoke<IEnumerable>(structure, Array.Empty<object>()); | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: extra blank line. |
||
var outliningSpans = new List<BlockSpan>(); | ||
var outliningSpans = new List<CodeFoldingBlock>(); | ||
foreach (var span in spans) | ||
{ | ||
if (_getIsCollpasible.Invoke<bool>(span, Array.Empty<object>())) | ||
{ | ||
var textSpan = _getTextSpan.Invoke<TextSpan>(span, Array.Empty<object>()); | ||
var line = lines.GetLineFromPosition(textSpan.Start); | ||
var column = textSpan.Start - line.Start; | ||
var endLine = lines.GetLineFromPosition(textSpan.End); | ||
var endColumn = textSpan.End - endLine.Start; | ||
|
||
outliningSpans.Add(new BlockSpan( | ||
textSpan.ToRange(lines), | ||
hintSpan: _getHintSpan.Invoke<TextSpan>(span, Array.Empty<object>()).ToRange(lines), | ||
bannerText: _getBannerText.Invoke<string>(span, Array.Empty<object>()), | ||
type: _getType.Invoke<string>(span, Array.Empty<object>()))); | ||
outliningSpans.Add(new CodeFoldingBlock( | ||
text.GetRangeFromSpan(textSpan), | ||
type: ConvertToWellKnownBlockType(_getType.Invoke<string>(span, Array.Empty<object>())))); | ||
} | ||
} | ||
|
||
return new BlockStructureResponse() { Spans = outliningSpans }; | ||
} | ||
|
||
|
||
private string ConvertToWellKnownBlockType(string kind) | ||
{ | ||
return kind == CodeFoldingBlockKind.Comment || | ||
kind == CodeFoldingBlockKind.Imports || | ||
kind == CodeFoldingBlockKind.Region | ||
? kind | ||
: null; | ||
} | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,6 @@ | |
using System.Threading.Tasks; | ||
using OmniSharp.Models.v2; | ||
using OmniSharp.Roslyn.CSharp.Services.Structure; | ||
using OmniSharp.Roslyn.Extensions; | ||
using TestUtility; | ||
using Xunit; | ||
using Xunit.Abstractions; | ||
|
@@ -33,26 +32,26 @@ void M()[| | |
var text = testFile.Content.Text; | ||
|
||
var lineSpans = (await GetResponseAsync(testFile)).Spans | ||
.Select(b => b.TextSpan) | ||
.Select(b => b.Range) | ||
.ToArray(); | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: extra blank line. |
||
var expected = testFile.Content.GetSpans() | ||
.Select(t => t.ToRange(text.Lines)).ToArray(); | ||
.Select(span => testFile.Content.GetRangeFromSpan(span).ToRange()).ToArray(); | ||
|
||
Assert.Equal(expected, lineSpans); | ||
} | ||
|
||
private async Task<BlockStructureResponse> GetResponseAsync(TestFile testFile) | ||
private Task<BlockStructureResponse> GetResponseAsync(TestFile testFile) | ||
{ | ||
SharedOmniSharpTestHost.AddFilesToWorkspace(new[] { testFile }); | ||
SharedOmniSharpTestHost.AddFilesToWorkspace(testFile); | ||
var request = new BlockStructureRequest | ||
{ | ||
FileName = testFile.FileName, | ||
}; | ||
|
||
var requestHandler = GetRequestHandler(SharedOmniSharpTestHost); | ||
return await requestHandler.Handle(request); | ||
return requestHandler.Handle(request); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind
?